Skip to content

report the capability of memory support - #514

Merged
XinfengZhang merged 1 commit into
intel:masterfrom
XinfengZhang:node_sel
Jun 8, 2021
Merged

report the capability of memory support#514
XinfengZhang merged 1 commit into
intel:masterfrom
XinfengZhang:node_sel

Conversation

@XinfengZhang

Copy link
Copy Markdown
Contributor

extend display attibute to include adaptor/hw/platform attribute
and add two new attributes to indicate whether there are on device memory
and whether there are different memory region on the device

Signed-off-by: Carl Zhang carl.zhang@intel.com

@XinfengZhang

Copy link
Copy Markdown
ContributorAuthor

it is another option for #503

Comment threadva/va.h Outdated
* attributes are supported by the driver, and then set the appropriate attributes
* before calling vaPutSurface()
*
* Display attributes also are used to query/set display adaptor (vaDisplay) related information

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. in the end of the sentence

Comment threadva/va.h Outdated
* before calling vaPutSurface()
*
* Display attributes also are used to query/set display adaptor (vaDisplay) related information
* These attributes does not depend on vaConfig, only related with platform or display adaptor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "do not".

'.' in the end of the sentence

Comment threadva/va.h Outdated
* App could call vaQueryDisplayAttributes/vaGetDisplayAttributes to query these attributes
* at anytime after vaInitialize, but only could call vaSetDisplayAttributes for these attributes
* after vaInitialize and before any other function call if the attributes is settable.
* To distinguish these two types display attributes, this display adaptor related attributres

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to types >>>of<<<

Comment threadva/va.h Outdated
*
* Display attributes also are used to query/set display adaptor (vaDisplay) related information
* These attributes does not depend on vaConfig, only related with platform or display adaptor
* It could not used for vaPutSurface.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type: >>>They<< could not >>>be<<< used

@XinfengZhang
XinfengZhangforce-pushed the node_sel branch 3 times, most recently from 82b8536 to 72fbd48CompareApril 12, 2021 14:13
Comment threadva/va.h Outdated
* They could not be used for vaPutSurface.
* App could call vaQueryDisplayAttributes/vaGetDisplayAttributes to query these attributes
* at anytime after vaInitialize, but only could call vaSetDisplayAttributes for these attributes
* after vaInitialize and before any other function call if the attributes is settable.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we modify the behavior to:

App could call vaQueryDisplayAttributes/vaGetDisplayAttributes to query these attributes at anytime after vaInitialize,
but only could call vaSetDisplayAttributes for these attributes BEFORE vaInitialize.

Which make the vaInitialize a time point to bind the tile?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we make the change just like your suggestion, it means that there are still no vaSetDisplayAttributes function pointer before initialize, part of the logic should in libva instead of backend driver.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, then I can accept this.

Comment threadva/va.h Outdated
* reg.value = reg_attr.value;
*
* for(int i = 0; i < reg.bits.local_memory_regions; i ++ ){
* if((1<<i) & reg.bits.memory_region){

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we use "reg.bits.memory_region_masks" instead?

Comment threadva/va.h Outdated
#define VA_RENDER_DEVICE_EXTERNAL 2

/** Memory info */
typedef union _VADisplayAttribValMemoryInfo{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this structure seems not necessary as we can derive if there is local memory or not from the query of VADisplayAttribValMemoryRegion. can we just remove it to make the interface concise?

@XinfengZhang
XinfengZhangforce-pushed the node_sel branch 3 times, most recently from 064500e to a1a2fb7CompareApril 22, 2021 08:22
Comment threadva/va.h Outdated
*}
* \endcode
*/
uint32_t memroy_region_masks : 16;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix typos: s/memroy/memory/. And should we use plural in a different way: memory_regions_mask?

Comment threadva/va.h
* called after vaInitialize and before any other function call.
*
* To distinguish these two types of display attributes, display adaptor related attributes
* should be marked as "HW attribute" in the description.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mean platform related information is not 'HW attribute' ?

How about to use VADisplayAdaptorAttrib instead of VADisplayAttrib as prefix?

Comment threadva/va.h Outdated
uint32_t memory_regions_masks : 16;
}bits;
uint32_t value;
}VADisplayAttribValMemoryRegion;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use VADisplayAdaptorAttrib as prefix ?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is good idea, do we need to change the VADisplayAttribSubDevice also, if we changed it, in the enum, the prefix looks not aligned.

Comment threadva/va.h Outdated
*}
* \endcode
*/
uint32_t memory_regions_masks : 16;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 16 enough for future use?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not , we could consider use min_value and max_value.

@XinfengZhang
XinfengZhangforce-pushed the node_sel branch 2 times, most recently from af5ef97 to 6d0c68eCompareJune 2, 2021 06:44
Comment threadva/va.h Outdated

/**\brief sub device info
* Sub-device is the concept basing on the "device" behind "vaDisplay".
* If a device could be divided to several sub device, the task of

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"to several sub devices", plural

Comment threadva/va.h Outdated
* If a device could be divided to several sub device, the task of
* decode/encode/vpp could be assigned on one sub-device. So, application
* could choose the sub device and attach it to vaDisplay before
* any other operations. after that, all of the task execution/resource

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"After", capital letter

Comment threadva/va.h Outdated
* Sub-device is the concept basing on the "device" behind "vaDisplay".
* If a device could be divided to several sub device, the task of
* decode/encode/vpp could be assigned on one sub-device. So, application
* could choose the sub device and attach it to vaDisplay before

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite like "attach". It sounds like in the future we might be able to attach multiple sub-devices to vaDisplay. Is that we want to have? This will require API change however since since right now we have only current_sub_device, but we will need a mask.

Comment threadva/va.h Outdated
*}
* \endcode
*/
uint32_t sub_devices_masks : 16;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too many plurals. I suggest sub_devices_mask

@XinfengZhang
XinfengZhangforce-pushed the node_sel branch 2 times, most recently from b396a9c to 969d7b8CompareJune 3, 2021 06:14
Extend display attibute to include adaptor/hw/platform attribute
and add new attributes to indicate whether there are several sub device behind vaDisplay
Signed-off-by: Carl Zhang <carl.zhang@intel.com>
@XinfengZhang
XinfengZhang merged commit 8803fc5 into intel:masterJun 8, 2021
Jexu pushed a commit to Jexu/media-driver that referenced this pull request May 21, 2025
* [Media Common] implement multiple tile selection interfaces
against intel/libva#514
add some comments for the init function
move the implementation of display attributes into MediaLibvaCaps class
implement VADisplayAttribSubDevice for some platforms
Jexu pushed a commit to Jexu/media-driver that referenced this pull request May 21, 2025
1. add VADisplayAttribSubDevice support
2. add comments for media context init
3. this pr is against: github.com/intel/libva/pull/514
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@XinfengZhang@HeJunyan@dvrogozh@xhaihao@liangali