[PATCH v2 1/4] drm: Adding new flag to restrict bitmask drm properties as 32 bit type and 32 bit value pair

List overview All Threads
Download

newer

older

[PATCH] MAINTAINERS: update...

[Bug 68805] New: 48 fps and 50 fps...

sagar.a.kamble＠intel.com

25 Mar 2014 25 Mar '14

2:32 p.m.

From: Sagar Kamble sagar.a.kamble@intel.com

With this patch new flag DRM_MODE_PROP_32BIT_PAIR is added that will help make use of 64 bit value of bitmask property as two 32 bit values.

Cc: airlied@linux.ie Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Sagar Kamble sagar.a.kamble@intel.com --- drivers/gpu/drm/drm_crtc.c | 22 ++++++++++++++++------ include/uapi/drm/drm_mode.h | 3 +++ 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 4e43fc2..d0d03ec 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2993,10 +2993,13 @@ int drm_property_add_enum(struct drm_property *property, int index,

/* * Bitmask enum properties have the additional constraint of values - * from 0 to 63 + * from 0 to 63. For properties with 32BIT_PAIR Flag set this constraint + * range is 0 to 31. */ - if ((property->flags & DRM_MODE_PROP_BITMASK) && (value > 63)) - return -EINVAL; + if (property->flags & DRM_MODE_PROP_BITMASK) + if (((property->flags & DRM_MODE_PROP_32BIT_PAIR) && (value > 31)) || + (value > 63)) + return -EINVAL;

if (!list_empty(&property->enum_blob_list)) { list_for_each_entry(prop_enum, &property->enum_blob_list, head) { @@ -3305,9 +3308,16 @@ static bool drm_property_change_is_valid(struct drm_property *property, } else if (property->flags & DRM_MODE_PROP_BITMASK) { int i; uint64_t valid_mask = 0; - for (i = 0; i < property->num_values; i++) - valid_mask |= (1ULL << property->values[i]); - return !(value & ~valid_mask); + uint32_t valid_32bit_mask = 0; + if (property->flags & DRM_MODE_PROP_32BIT_PAIR) { + for (i = 0; i < property->num_values; i++) + valid_32bit_mask |= (1UL << property->values[i]); + return !((value & 0xFFFFFFFF) & ~valid_32bit_mask); + } else { + for (i = 0; i < property->num_values; i++) + valid_mask |= (1ULL << property->values[i]); + return !(value & ~valid_mask); + } } else if (property->flags & DRM_MODE_PROP_BLOB) { /* Only the driver knows */ return true; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index f104c26..5e3a7d9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -250,6 +250,9 @@ struct drm_mode_get_connector { #define DRM_MODE_PROP_ENUM (1<<3) /* enumerated type with text strings */ #define DRM_MODE_PROP_BLOB (1<<4) #define DRM_MODE_PROP_BITMASK (1<<5) /* bitmask of enumerated types */ +#define DRM_MODE_PROP_32BIT_PAIR (1<<6) /* 32 bit bitmask of enumerated types + * and 32 bit of value of the type */ +

struct drm_mode_property_enum { __u64 value;

-- 1.8.5

Show replies by date

sagar.a.kamble＠intel.com

25 Mar 25 Mar

2:32 p.m.

New subject: [PATCH v2 2/4] drm: Added plane alpha and color blending property

From: Sagar Kamble sagar.a.kamble@intel.com

This patch creates a generic blending bitmask property modeled after glBlendFunc. Drivers may support subset of these values.

v2: Removing blend properties that are not applicable [Damien's Review Comments]. Adding DRM_MODE_PROP_32BIT_PAIR flag to blend property.

Cc: airlied@linux.ie Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Sagar Kamble sagar.a.kamble@intel.com --- drivers/gpu/drm/drm_crtc.c | 27 +++++++++++++++++++++++++++ include/drm/drm_crtc.h | 19 +++++++++++++++++++ 2 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d0d03ec..a1f254e 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4157,3 +4157,30 @@ void drm_mode_config_cleanup(struct drm_device *dev) idr_destroy(&dev->mode_config.crtc_idr); } EXPORT_SYMBOL(drm_mode_config_cleanup); + +struct drm_property *drm_mode_create_blend_property(struct drm_device *dev, + unsigned int supported_factors) +{ + static const struct drm_prop_enum_list props[] = { + { DRM_BLEND_ZERO, "zero" }, + { DRM_BLEND_ONE, "one" }, + { DRM_BLEND_SRC_COLOR, "src-color" }, + { DRM_BLEND_ONE_MINUS_SRC_COLOR, "one-minus-src-color" }, + { DRM_BLEND_DST_COLOR, "dst-color" }, + { DRM_BLEND_ONE_MINUS_DST_COLOR, "one-minus-dst-color" }, + { DRM_BLEND_SRC_ALPHA, "src-alpha" }, + { DRM_BLEND_ONE_MINUS_SRC_ALPHA, "one-minus-src-alpha" }, + { DRM_BLEND_DST_ALPHA, "dst-alpha" }, + { DRM_BLEND_ONE_MINUS_DST_ALPHA, "one-minus-dst-alpha" }, + { DRM_BLEND_CONSTANT_COLOR, "constant-color" }, + { DRM_BLEND_ONE_MINUS_CONSTANT_COLOR, "one-minus-constant-color" }, + { DRM_BLEND_CONSTANT_ALPHA, "constant-alpha" }, + { DRM_BLEND_ONE_MINUS_CONSTANT_ALPHA, "one-minus-constant-alpha" }, + { DRM_BLEND_SRC_ALPHA_SATURATE, "alpha-saturate" } + }; + + return drm_property_create_bitmask(dev, DRM_MODE_PROP_32BIT_PAIR, "blend", + props, ARRAY_SIZE(props), + supported_factors); +} +EXPORT_SYMBOL(drm_mode_create_blend_property); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 784a568..a9fbfec 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -65,6 +65,23 @@ struct drm_object_properties { uint64_t values[DRM_OBJECT_MAX_PROPERTY]; };

+/* Blending property bits */ +#define DRM_BLEND_ZERO 0 +#define DRM_BLEND_ONE 1 +#define DRM_BLEND_SRC_COLOR 2 +#define DRM_BLEND_ONE_MINUS_SRC_COLOR 3 +#define DRM_BLEND_DST_COLOR 4 +#define DRM_BLEND_ONE_MINUS_DST_COLOR 5 +#define DRM_BLEND_SRC_ALPHA 6 +#define DRM_BLEND_ONE_MINUS_SRC_ALPHA 7 +#define DRM_BLEND_DST_ALPHA 8 +#define DRM_BLEND_ONE_MINUS_DST_ALPHA 9 +#define DRM_BLEND_CONSTANT_COLOR 10 +#define DRM_BLEND_ONE_MINUS_CONSTANT_COLOR 11 +#define DRM_BLEND_CONSTANT_ALPHA 12 +#define DRM_BLEND_ONE_MINUS_CONSTANT_ALPHA 13 +#define DRM_BLEND_SRC_ALPHA_SATURATE 14 + /* * Note on terminology: here, for brevity and convenience, we refer to connector * control chips as 'CRTCs'. They can control any type of connector, VGA, LVDS, @@ -1179,6 +1196,8 @@ extern int drm_format_plane_cpp(uint32_t format, int plane); extern int drm_format_horz_chroma_subsampling(uint32_t format); extern int drm_format_vert_chroma_subsampling(uint32_t format); extern const char *drm_get_format_name(uint32_t format); +extern struct drm_property *drm_mode_create_blend_property(struct drm_device *dev, + unsigned int supported_factors);

/* Helpers */ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,

-- 1.8.5

sagar.a.kamble＠intel.com

2:32 p.m.

New subject: [PATCH v2 4/4] Documentation: drm: describing plane alpha and color blending property

From: Sagar Kamble sagar.a.kamble@intel.com

v2: Added description for "src-color" and "constant-alpha" property.

Cc: Rob Landley <rob at landley.net> Cc: Dave Airlie <airlied at redhat.com> Cc: Daniel Vetter <daniel.vetter at ffwll.ch> Cc: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com> Cc: David Herrmann <dh.herrmann at gmail.com> Cc: Alex Deucher <alexander.deucher at amd.com> Cc: "Ville Syrjälä" <ville.syrjala at linux.intel.com> Cc: Sagar Kamble <sagar.a.kamble at intel.com> Cc: "Purushothaman, Vijay A" <vijay.a.purushothaman at intel.com> Cc: linux-doc at vger.kernel.org Cc: dri-devel at lists.freedesktop.org Signed-off-by: Sagar Kamble sagar.a.kamble@intel.com --- Documentation/DocBook/drm.tmpl | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 104402a..77a45fb 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2253,6 +2253,14 @@ void intel_crt_init(struct drm_device *dev) enumerated bits defined by the property.</para></listitem> </varlistentry> <varlistentry> + <term>DRM_MODE_PROP_32BIT_PAIR</term> + <listitem><para>This flag restricts Bitmask properties restricts all + enumerated values to the 0..31 range. + During get operation instance values combine one or more of the + enumerated bits defined by the property. During get user can specify + {type, value} pair.</para></listitem> + </varlistentry> + <varlistentry> <term>DRM_MODE_PROP_BLOB</term> <listitem><para>Blob properties store a binary blob without any format restriction. The binary blobs are created as KMS standalone objects, @@ -2336,7 +2344,7 @@ void intel_crt_init(struct drm_device *dev) </tr> <tr> <td rowspan="19" valign="top" >DRM</td> - <td rowspan="2" valign="top" >Generic</td> + <td rowspan="3" valign="top" >Generic</td> <td valign="top" >“EDID”</td> <td valign="top" >BLOB | IMMUTABLE</td> <td valign="top" >0</td> @@ -2351,6 +2359,19 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >Contains DPMS operation mode value.</td> </tr> <tr> + <td valign="top" >“blend”</td> + <td valign="top" >BITMASK | 32BIT_PAIR</td> + <td valign="top" >{ {0, "zero"}, {1, "one"}, {2, "src-color"}, {3, "one-minus-src-color"} + , {4, "dst-color"}, {5, "one-minus-dst-color"}, {6, "src-alpha"}, {7, "one-minus-src-alpha"}, {8, "dst-alpha"} + , {9, "one-minus-dst-alpha"}, {10, "constant-color"}, {11, "one-minus-constant-color"}, {12, "constant-alpha"} + , {13, "one-minus-constant-alpha"}, {14, "alpha-saturate"} }</td> + <td valign="top" >Plane</td> + <td valign="top" >Contains plane alpha/color blending operation values. These are modeled after glBlendFunc API + in OpenGL. Currently only "src-color" and "constant-alpha" are supported. <para>"src-color" will consider supplied fb + with plane's pixel format as input without any additional color/alpha changes.</para><para>"constant-alpha" will apply constant + transparency to all pixels in addition to source color.</para></td> + </tr> + <tr> <td rowspan="2" valign="top" >DVI-I</td> <td valign="top" >“subconnector”</td> <td valign="top" >ENUM</td>

-- 1.8.5

David Herrmann

26 Mar 26 Mar

12:30 p.m.

New subject: [PATCH v2 4/4] Documentation: drm: describing plane alpha and color blending property

On Tue, Mar 25, 2014 at 3:32 PM, sagar.a.kamble@intel.com wrote:

...

From: Sagar Kamble sagar.a.kamble@intel.com

v2: Added description for "src-color" and "constant-alpha" property.

Cc: Rob Landley <rob at landley.net> Cc: Dave Airlie <airlied at redhat.com> Cc: Daniel Vetter <daniel.vetter at ffwll.ch> Cc: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com> Cc: David Herrmann <dh.herrmann at gmail.com> Cc: Alex Deucher <alexander.deucher at amd.com> Cc: "Ville Syrjälä" <ville.syrjala at linux.intel.com> Cc: Sagar Kamble <sagar.a.kamble at intel.com> Cc: "Purushothaman, Vijay A" <vijay.a.purushothaman at intel.com> Cc: linux-doc at vger.kernel.org Cc: dri-devel at lists.freedesktop.org Signed-off-by: Sagar Kamble sagar.a.kamble@intel.com

Documentation/DocBook/drm.tmpl | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 104402a..77a45fb 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2253,6 +2253,14 @@ void intel_crt_init(struct drm_device *dev) enumerated bits defined by the property.</para></listitem> </varlistentry> <varlistentry>
     <term>DRM_MODE_PROP_32BIT_PAIR</term>
     <listitem><para>This flag restricts Bitmask properties restricts all
      enumerated values to the 0..31 range.
       During get operation instance values combine one or more of the
       enumerated bits defined by the property. During get user can specify
      {type, value} pair.</para></listitem>

Please rewrite that, lots of typos in there. Furthermore, if you use verbs as nouns, you should emphasize them. So in your text, please somehow emphasize/quote 'get'.

Thanks David

...

```
   </varlistentry>
```

   <varlistentry>
     <term>DRM_MODE_PROP_BLOB</term>
     <listitem><para>Blob properties store a binary blob without any format
       restriction. The binary blobs are created as KMS standalone objects,

@@ -2336,7 +2344,7 @@ void intel_crt_init(struct drm_device *dev) </tr> <tr> <td rowspan="19" valign="top" >DRM</td>

  <td rowspan="2" valign="top" >Generic</td>

  <td rowspan="3" valign="top" >Generic</td>
  <td valign="top" >"EDID"</td>
  <td valign="top" >BLOB | IMMUTABLE</td>
  <td valign="top" >0</td>

@@ -2351,6 +2359,19 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >Contains DPMS operation mode value.</td> </tr> <tr>

```
  <td valign="top" >"blend"</td>
```

  <td valign="top" >BITMASK | 32BIT_PAIR</td>

  <td valign="top" >{ {0, "zero"}, {1, "one"}, {2, "src-color"}, {3, "one-minus-src-color"}

  , {4, "dst-color"}, {5, "one-minus-dst-color"}, {6, "src-alpha"}, {7, "one-minus-src-alpha"}, {8, "dst-alpha"}

  , {9, "one-minus-dst-alpha"}, {10, "constant-color"}, {11, "one-minus-constant-color"}, {12, "constant-alpha"}

  , {13, "one-minus-constant-alpha"}, {14, "alpha-saturate"} }</td>

```
  <td valign="top" >Plane</td>
```

  <td valign="top" >Contains plane alpha/color blending operation values. These are modeled after glBlendFunc API

  in OpenGL. Currently only "src-color" and "constant-alpha" are supported. <para>"src-color" will consider supplied fb

  with plane's pixel format as input without any additional color/alpha changes.</para><para>"constant-alpha" will apply constant

  transparency to all pixels in addition to source color.</para></td>

```
  </tr>
```

  <tr>
  <td rowspan="2" valign="top" >DVI-I</td>
  <td valign="top" >"subconnector"</td>
  <td valign="top" >ENUM</td>

-- 1.8.5

sagar.a.kamble＠intel.com

27 Mar 27 Mar

9:03 a.m.

New subject: [PATCH v3 1/1] Documentation: drm: describing plane alpha and color blending property

From: Sagar Kamble sagar.a.kamble@intel.com

v2: Added description for "src-color" and "constant-alpha" property. [Review by Laurent Pinchart]

v3: Fixed typos. [Review by David Hermann]

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 104402a..66386d0 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2253,6 +2253,14 @@ void intel_crt_init(struct drm_device *dev) enumerated bits defined by the property.</para></listitem> </varlistentry> <varlistentry> + <term>DRM_MODE_PROP_32BIT_PAIR</term> + <listitem><para>This flag restricts all enumerated values of Bitmask properties + to the 0..31 range. + 'get_property' operation will get combination of instance values of one or more of the + enumerated bits defined by the property. With 'set_property' operation, user can specify + {value (MSB 32 bits), type (LSB 32 bits)} pair with 64 bit value for that property.</para></listitem> + </varlistentry> + <varlistentry> <term>DRM_MODE_PROP_BLOB</term> <listitem><para>Blob properties store a binary blob without any format restriction. The binary blobs are created as KMS standalone objects, @@ -2336,7 +2344,7 @@ void intel_crt_init(struct drm_device *dev) </tr> <tr> <td rowspan="19" valign="top" >DRM</td> - <td rowspan="2" valign="top" >Generic</td> + <td rowspan="3" valign="top" >Generic</td> <td valign="top" >“EDID”</td> <td valign="top" >BLOB | IMMUTABLE</td> <td valign="top" >0</td> @@ -2351,6 +2359,19 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >Contains DPMS operation mode value.</td> </tr> <tr> + <td valign="top" >“blend”</td> + <td valign="top" >BITMASK | 32BIT_PAIR</td> + <td valign="top" >{ {0, "zero"}, {1, "one"}, {2, "src-color"}, {3, "one-minus-src-color"} + , {4, "dst-color"}, {5, "one-minus-dst-color"}, {6, "src-alpha"}, {7, "one-minus-src-alpha"}, {8, "dst-alpha"} + , {9, "one-minus-dst-alpha"}, {10, "constant-color"}, {11, "one-minus-constant-color"}, {12, "constant-alpha"} + , {13, "one-minus-constant-alpha"}, {14, "alpha-saturate"} }</td> + <td valign="top" >Plane</td> + <td valign="top" >Contains plane alpha/color blending operation values. These are modeled after glBlendFunc API + in OpenGL. Currently only "src-color" and "constant-alpha" are supported. <para>"src-color" will consider supplied fb + with plane's pixel format as input without any additional color/alpha changes.</para><para>"constant-alpha" will apply constant + transparency to all pixels in addition to source color.</para></td> + </tr> + <tr> <td rowspan="2" valign="top" >DVI-I</td> <td valign="top" >“subconnector”</td> <td valign="top" >ENUM</td>

-- 1.8.5

sagar.a.kamble＠intel.com

9:50 a.m.

New subject: [PATCH v3 1/1] Documentation: drm: describing plane alpha and color blending property

From: Sagar Kamble sagar.a.kamble@intel.com

v2: Added description for "src-color" and "constant-alpha" property. [Review by Laurent Pinchart]

v3: Fixed typos. [Review by David Herrmann]

-- 1.8.5

David Herrmann

12:38 p.m.

New subject: [PATCH v3 1/1] Documentation: drm: describing plane alpha and color blending property

On Thu, Mar 27, 2014 at 10:50 AM, sagar.a.kamble@intel.com wrote:

...

From: Sagar Kamble sagar.a.kamble@intel.com

v2: Added description for "src-color" and "constant-alpha" property. [Review by Laurent Pinchart]

v3: Fixed typos. [Review by David Herrmann]

Cc: Rob Landley <rob at landley.net> Cc: Dave Airlie <airlied at redhat.com> Cc: Daniel Vetter <daniel.vetter at ffwll.ch> Cc: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com> Cc: David Herrmann <dh.herrmann at gmail.com> Cc: Alex Deucher <alexander.deucher at amd.com> Cc: "Ville Syrjälä" <ville.syrjala at linux.intel.com> Cc: Sagar Kamble <sagar.a.kamble at intel.com> Cc: "Purushothaman, Vijay A" <vijay.a.purushothaman at intel.com> Cc: linux-doc at vger.kernel.org Cc: dri-devel at lists.freedesktop.org Signed-off-by: Sagar Kamble sagar.a.kamble@intel.com

Documentation/DocBook/drm.tmpl | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 104402a..66386d0 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2253,6 +2253,14 @@ void intel_crt_init(struct drm_device *dev) enumerated bits defined by the property.</para></listitem> </varlistentry> <varlistentry>
     <term>DRM_MODE_PROP_32BIT_PAIR</term>
     <listitem><para>This flag restricts all enumerated values of Bitmask properties
      to the 0..31 range.
       'get_property' operation will get combination of instance values of one or more of the
       enumerated bits defined by the property. With 'set_property' operation, user can specify
      {value (MSB 32 bits), type (LSB 32 bits)} pair with 64 bit value for that property.</para></listitem>

This is a type description, right? I think this is overly complex (and I actually don't understand it). How about this:

"Pair-properties store two packed 32 bit values as 64bit unsigned integer (often used as key-value pair). The lower 32bits contain the first value (also: key), while the upper 32bits contain the second value. Written as: ({key (LSB 32 bits), value (MSB 32 bits)})."

That should be enough for anyone to understand it. Please note that I inverted 'type' and 'value' here. It's unnatural to list the value first.

...

   </varlistentry>
   <varlistentry>
     <term>DRM_MODE_PROP_BLOB</term>
     <listitem><para>Blob properties store a binary blob without any format
       restriction. The binary blobs are created as KMS standalone objects,
@@ -2336,7 +2344,7 @@ void intel_crt_init(struct drm_device *dev) </tr> <tr> <td rowspan="19" valign="top" >DRM</td>
  <td rowspan="2" valign="top" >Generic</td>
  <td rowspan="3" valign="top" >Generic</td>
  <td valign="top" >"EDID"</td>
  <td valign="top" >BLOB | IMMUTABLE</td>
  <td valign="top" >0</td>
@@ -2351,6 +2359,19 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >Contains DPMS operation mode value.</td> </tr> <tr>
  <td valign="top" >"blend"</td>
  <td valign="top" >BITMASK | 32BIT_PAIR</td>

Ugh? This is not a bitmask, is it? It's just a 32BIT_PAIR, right?

...

  <td valign="top" >{ {0, "zero"}, {1, "one"}, {2, "src-color"}, {3, "one-minus-src-color"}

  , {4, "dst-color"}, {5, "one-minus-dst-color"}, {6, "src-alpha"}, {7, "one-minus-src-alpha"}, {8, "dst-alpha"}

  , {9, "one-minus-dst-alpha"}, {10, "constant-color"}, {11, "one-minus-constant-color"}, {12, "constant-alpha"}

  , {13, "one-minus-constant-alpha"}, {14, "alpha-saturate"} }</td>

I think brackets are weird to describe enums, a table would be nicer. People might misread this as 32-bit pairs, which this isn't. If you want to avoid writing yet another table, how about:

ZERO = 0, ONE = 1, SRC_COLOR = 2, .. and so on?

...

```
  <td valign="top" >Plane</td>
```

  <td valign="top" >Contains plane alpha/color blending operation values. These are modeled after glBlendFunc API

Remove 'API' (or add 'the', like 'the glBlendFunc API').

...

  in OpenGL. Currently only "src-color" and "constant-alpha" are supported. <para>"src-color" will consider supplied fb

This is a driver-dependent statement. I don't think it belongs here. I'd rather write it as "Not all modes are supported by all drivers.".

Also, please do line-breaks in front of <para>. They start new-paragraphs, so we should do the same in the code (same below).

...

  with plane's pixel format as input without any additional color/alpha changes.</para><para>"constant-alpha" will apply constant

  transparency to all pixels in addition to source color.</para></td>

I would just drop the description of each of the modes. They're described in the man-pages of glBlendFunc and everyone knows them (and they're fairly obvious).

However, this doc doesn't describe the second value at all? glBlendFunc() takes two of these enums, one for the source, one for the target. So please add some statement like:

"The first value of the 32BIT_PAIR describes the blending factors of the source image, the second value describes the blending factors of the destination image."

Thanks David

...

```
  </tr>
```

  <tr>
  <td rowspan="2" valign="top" >DVI-I</td>
  <td valign="top" >"subconnector"</td>
  <td valign="top" >ENUM</td>

-- 1.8.5

sagar.a.kamble＠intel.com

5:31 p.m.

New subject: [PATCH v4 1/1] Documentation: drm: describing plane alpha and color blending property

From: Sagar Kamble sagar.a.kamble@intel.com

v2: Added description for "src-color" and "constant-alpha" property. [Review by Laurent Pinchart]

v3: Fixed typos. [Review by David Herrmann]

v4: Additional formatting and modified description. [Review by David Herrmann]

Cc: rob@landley.net Cc: airlied@redhat.com Cc: daniel.vetter@ffwll.ch Cc: laurent.pinchart@ideasonboard.com Cc: dh.herrmann@gmail.com Cc: alexander.deucher@amd.com Cc: ville.syrjala@linux.intel.com Cc: sagar.a.kamble@intel.com Cc: vijay.a.purushothaman@intel.com Cc: linux-doc@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Sagar Kamble sagar.a.kamble@intel.com --- Documentation/DocBook/drm.tmpl | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 104402a..dd2ae67 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2264,6 +2264,20 @@ void intel_crt_init(struct drm_device *dev) </variablelist> </para> <para> + In addition to above types, properties can be constrained by supplying additional + flags while creating the property. Supported flags are + <variablelist> + <varlistentry> + <term>DRM_MODE_PROP_32BIT_PAIR</term> + <listitem><para>This flag is applicable to Bitmask enum properties. It creates + Pair-properties that store two packed 32 bit values as 64bit unsigned integer + (often used as key-value pair). The lower 32 bits contain the first value (also: key), + while the upper 32 bits contain the second value. + Written as: ({key (LSB 32 bits), value (MSB 32 bits)}).</para></listitem> + </varlistentry> + </variablelist> + </para> + <para> To create a property drivers call one of the following functions depending on the property type. All property creation functions take property flags and name, as well as type-specific arguments. @@ -2336,7 +2350,7 @@ void intel_crt_init(struct drm_device *dev) </tr> <tr> <td rowspan="19" valign="top" >DRM</td> - <td rowspan="2" valign="top" >Generic</td> + <td rowspan="3" valign="top" >Generic</td> <td valign="top" >“EDID”</td> <td valign="top" >BLOB | IMMUTABLE</td> <td valign="top" >0</td> @@ -2351,6 +2365,22 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >Contains DPMS operation mode value.</td> </tr> <tr> + <td valign="top" >“blend”</td> + <td valign="top" >BITMASK | 32BIT_PAIR</td> + <td valign="top" >{ "zero" = 0, "one" = 1, "src-color" = 2, "one-minus-src-color" = 3 + , "dst-color" = 4, "one-minus-dst-color" = 5, "src-alpha"= 6, "one-minus-src-alpha" = 7, "dst-alpha" = 8 + , "one-minus-dst-alpha" = 9, "constant-color" = 10, "one-minus-constant-color" = 11, "constant-alpha" = 12 + , "one-minus-constant-alpha" = 13, "alpha-saturate" = 14 }</td> + <td valign="top" >Plane</td> + <td valign="top" >Contains plane alpha/color blending operation values. These are modeled after the glBlendFunc API + in OpenGL. Not all modes are supported by all drivers. The first value of the 32BIT_PAIR describes the blending mode + of the source plane, the second value describes the blending factors of the source plane. + <para> + For e.g. to apply constant alpha of 0xFF on source plane, the first value of this property for source plane should + be set to 2^10 and second value to value of alpha(0xFF). + </para></td> + </tr> + <tr> <td rowspan="2" valign="top" >DVI-I</td> <td valign="top" >“subconnector”</td> <td valign="top" >ENUM</td>

-- 1.8.5

Sagar Arun Kamble

10 Apr 10 Apr

10:39 a.m.

Adding Rob and Rusty in the review thread.

Kindly review these patches for interface being proposed to set color/alpha property of planes modeled after glBlendFunc.

http://lists.freedesktop.org/archives/intel-gfx/2014-March/042350.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042351.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042352.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042587.html http://lists.freedesktop.org/archives/intel-gfx/2014-March/042354.html

thanks, Sagar

On Tue, 2014-03-25 at 20:02 +0530, sagar.a.kamble@intel.com wrote:

...

From: Sagar Kamble sagar.a.kamble@intel.com

With this patch new flag DRM_MODE_PROP_32BIT_PAIR is added that will help make use of 64 bit value of bitmask property as two 32 bit values.

Cc: airlied@linux.ie Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Sagar Kamble sagar.a.kamble@intel.com

drivers/gpu/drm/drm_crtc.c | 22 ++++++++++++++++------ include/uapi/drm/drm_mode.h | 3 +++ 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 4e43fc2..d0d03ec 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2993,10 +2993,13 @@ int drm_property_add_enum(struct drm_property *property, int index,

/* * Bitmask enum properties have the additional constraint of values
* from 0 to 63
* from 0 to 63. For properties with 32BIT_PAIR Flag set this constraint
* range is 0 to 31.
*/
if ((property->flags & DRM_MODE_PROP_BITMASK) && (value > 63))
return -EINVAL;
if (property->flags & DRM_MODE_PROP_BITMASK)
if (((property->flags & DRM_MODE_PROP_32BIT_PAIR) && (value > 31)) ||
    (value > 63))
	return -EINVAL;
if (!list_empty(&property->enum_blob_list)) { list_for_each_entry(prop_enum, &property->enum_blob_list, head) {
@@ -3305,9 +3308,16 @@ static bool drm_property_change_is_valid(struct drm_property *property, } else if (property->flags & DRM_MODE_PROP_BITMASK) { int i; uint64_t valid_mask = 0;
for (i = 0; i < property->num_values; i++)
	valid_mask |= (1ULL << property->values[i]);
return !(value & ~valid_mask);
uint32_t valid_32bit_mask = 0;
if (property->flags & DRM_MODE_PROP_32BIT_PAIR) {
	for (i = 0; i < property->num_values; i++)
		valid_32bit_mask |= (1UL << property->values[i]);
	return !((value & 0xFFFFFFFF) & ~valid_32bit_mask);
} else {
	for (i = 0; i < property->num_values; i++)
		valid_mask |= (1ULL << property->values[i]);
	return !(value & ~valid_mask);
}
} else if (property->flags & DRM_MODE_PROP_BLOB) { /* Only the driver knows */ return true;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index f104c26..5e3a7d9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -250,6 +250,9 @@ struct drm_mode_get_connector { #define DRM_MODE_PROP_ENUM (1<<3) /* enumerated type with text strings */ #define DRM_MODE_PROP_BLOB (1<<4) #define DRM_MODE_PROP_BITMASK (1<<5) /* bitmask of enumerated types */ +#define DRM_MODE_PROP_32BIT_PAIR (1<<6) /* 32 bit bitmask of enumerated types
			 * and 32 bit of value of the type */
struct drm_mode_property_enum { __u64 value;

4037

Age (days ago)

4053

Last active (days ago)

dri-devel@lists.freedesktop.org

8 comments

3 participants

tags (0)

participants (3)

David Herrmann
Sagar Arun Kamble
sagar.a.kamble＠intel.com