report the capability of memory support #514

XinfengZhang · 2021-03-31T13:35:31Z

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 · 2021-03-31T13:36:15Z

it is another option for #503

dvrogozh · 2021-04-09T23:45:28Z

va/va.h

@@ -4828,7 +4828,17 @@ VAStatus vaDeassociateSubpicture (
 * brightness etc. in the rendering process.  The application can query what
 * 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


. in the end of the sentence

dvrogozh · 2021-04-09T23:45:52Z

va/va.h

@@ -4828,7 +4828,17 @@ VAStatus vaDeassociateSubpicture (
 * brightness etc. in the rendering process.  The application can query what
 * 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
+ * These attributes does not depend on vaConfig, only related with platform or display adaptor


typo: "do not".

'.' in the end of the sentence

dvrogozh · 2021-04-09T23:47:06Z

va/va.h

+ * 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


to types >>>of<<<

dvrogozh · 2021-04-09T23:47:56Z

va/va.h

+ *
+ * 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.


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

HeJunyan · 2021-04-13T09:03:13Z

va/va.h

+ * 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.


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?

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.

OK, then I can accept this.

liangali · 2021-04-22T05:40:23Z

va/va.h

+         * reg.value = reg_attr.value;
+         *
+         * for(int i = 0; i < reg.bits.local_memory_regions; i ++ ){
+         *    if((1<<i) & reg.bits.memory_region){


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

liangali · 2021-04-22T05:55:17Z

va/va.h

@@ -4885,6 +4895,44 @@ typedef enum
 #define VA_RENDER_DEVICE_LOCAL      1
 #define VA_RENDER_DEVICE_EXTERNAL   2

+/** Memory info */
+typedef union _VADisplayAttribValMemoryInfo{


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?

dvrogozh · 2021-05-17T14:04:49Z

va/va.h

+         *}
+         * \endcode
+         */
+        uint32_t memroy_region_masks : 16;


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

xhaihao · 2021-05-26T03:09:42Z

va/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.


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

How about to use VADisplayAdaptorAttrib instead of VADisplayAttrib as prefix?

xhaihao · 2021-05-26T03:10:40Z

va/va.h

+        uint32_t memory_regions_masks : 16;
+    }bits;
+    uint32_t value;
+}VADisplayAttribValMemoryRegion;


Use VADisplayAdaptorAttrib as prefix ?

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

xhaihao · 2021-05-26T03:12:53Z

va/va.h

+         *}
+         * \endcode
+         */
+        uint32_t memory_regions_masks : 16;


Is 16 enough for future use?

if not , we could consider use min_value and max_value.

dvrogozh · 2021-06-02T17:14:31Z

va/va.h

@@ -4929,6 +4939,47 @@ typedef enum
 #define VA_RENDER_DEVICE_LOCAL      1
 #define VA_RENDER_DEVICE_EXTERNAL   2

+/**\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


"to several sub devices", plural

dvrogozh · 2021-06-02T17:16:05Z

va/va.h

+ * 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


"After", capital letter

dvrogozh · 2021-06-02T17:18:13Z

va/va.h

+ * 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


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.

dvrogozh · 2021-06-02T17:18:38Z

va/va.h

+         *}
+         * \endcode
+         */
+        uint32_t sub_devices_masks      : 16;


too many plurals. I suggest sub_devices_mask

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>

* [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

1. add VADisplayAttribSubDevice support 2. add comments for media context init 3. this pr is against: github.com/intel/libva/pull/514

XinfengZhang requested review from dvrogozh, xhaihao and xuguangxin March 31, 2021 13:36

emersion mentioned this pull request Apr 8, 2021

Introduce VADisplayAttribSurfaceAttribs #519

Closed

XinfengZhang force-pushed the node_sel branch 2 times, most recently from b7ffba9 to 48ba454 Compare April 9, 2021 12:49

dvrogozh reviewed Apr 9, 2021

View reviewed changes

XinfengZhang force-pushed the node_sel branch 3 times, most recently from 82b8536 to 72fbd48 Compare April 12, 2021 14:13

dvrogozh approved these changes Apr 12, 2021

View reviewed changes

dvrogozh added the ready label Apr 12, 2021

HeJunyan reviewed Apr 13, 2021

View reviewed changes

liangali reviewed Apr 22, 2021

View reviewed changes

XinfengZhang force-pushed the node_sel branch 3 times, most recently from 064500e to a1a2fb7 Compare April 22, 2021 08:22

liangali approved these changes Apr 23, 2021

View reviewed changes

XinfengZhang force-pushed the node_sel branch from a1a2fb7 to 4129b17 Compare April 29, 2021 06:42

dvrogozh approved these changes Apr 29, 2021

View reviewed changes

XinfengZhang force-pushed the node_sel branch from 4129b17 to b7655a4 Compare May 8, 2021 08:01

dvrogozh reviewed May 17, 2021

View reviewed changes

XinfengZhang force-pushed the node_sel branch from b7655a4 to e0596f0 Compare May 19, 2021 13:39

xhaihao reviewed May 26, 2021

View reviewed changes

XinfengZhang force-pushed the node_sel branch from e0596f0 to e05e029 Compare June 2, 2021 06:06

XinfengZhang force-pushed the node_sel branch 2 times, most recently from af5ef97 to 6d0c68e Compare June 2, 2021 06:44

dvrogozh requested changes Jun 2, 2021

View reviewed changes

XinfengZhang force-pushed the node_sel branch 2 times, most recently from b396a9c to 969d7b8 Compare June 3, 2021 06:14

report the capability of sub device

969d7b8

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>

dvrogozh approved these changes Jun 7, 2021

View reviewed changes

XinfengZhang merged commit 8803fc5 into intel:master Jun 8, 2021

Jexu pushed a commit to Jexu/media-driver that referenced this pull request May 21, 2025

[Media Common]enable VADisplayAttribSubDevice support

b17cfd2

1. add VADisplayAttribSubDevice support 2. add comments for media context init 3. this pr is against: github.com/intel/libva/pull/514

report the capability of memory support #514

report the capability of memory support #514

Uh oh!

Conversation

XinfengZhang commented Mar 31, 2021

Uh oh!

XinfengZhang commented Mar 31, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!