diff mbox

[12/14] drm/i915: Add NV12 as supported format for primary plane

Message ID 1507637878-17165-13-git-send-email-vidya.srinivas@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Srinivas, Vidya Oct. 10, 2017, 12:17 p.m. UTC 
From: Chandra Konduru <chandra.konduru@intel.com>

This patch adds NV12 to list of supported formats for
primary plane

v2: Rebased (Chandra Konduru)

v3: Rebased (me)

v4: Review comments by Ville addressed
	Removed the skl_primary_formats_with_nv12 and
	added NV12 case in existing skl_primary_formats

v5: Rebased (me)

v6: Missed the Tested-by/Reviewed-by in the previous series
	Adding the same to commit message in this version.

v7: Review comments by Ville addressed
	Restricting the NV12 for BXT and on PIPE A and B
	Rebased (me)

v8: Rebased (me)
	Modified restricting the NV12 support for both BXT and KBL.

Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

Comments

Kumar, Mahesh Oct. 10, 2017, 2:09 p.m. UTC | #1 
Hi,


On Tuesday 10 October 2017 05:47 PM, Vidya Srinivas wrote:
> From: Chandra Konduru <chandra.konduru@intel.com>
>
> This patch adds NV12 to list of supported formats for
> primary plane
>
> v2: Rebased (Chandra Konduru)
>
> v3: Rebased (me)
>
> v4: Review comments by Ville addressed
> 	Removed the skl_primary_formats_with_nv12 and
> 	added NV12 case in existing skl_primary_formats
>
> v5: Rebased (me)
>
> v6: Missed the Tested-by/Reviewed-by in the previous series
> 	Adding the same to commit message in this version.
>
> v7: Review comments by Ville addressed
> 	Restricting the NV12 for BXT and on PIPE A and B
> 	Rebased (me)
>
> v8: Rebased (me)
> 	Modified restricting the NV12 support for both BXT and KBL.
>
> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++--
>   1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f71a704..e551f59 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -106,6 +106,22 @@
>   	DRM_FORMAT_MOD_INVALID
>   };
>   
> +static const uint32_t nv12_primary_formats[] = {
> +	DRM_FORMAT_C8,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XRGB2101010,
> +	DRM_FORMAT_XBGR2101010,
> +	DRM_FORMAT_YUYV,
> +	DRM_FORMAT_YVYU,
> +	DRM_FORMAT_UYVY,
> +	DRM_FORMAT_VYUY,
> +	DRM_FORMAT_NV12,
> +};
> +
>   /* Cursor formats */
>   static const uint32_t intel_cursor_formats[] = {
>   	DRM_FORMAT_ARGB8888,
> @@ -13256,8 +13272,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
>   		primary->update_plane = skylake_update_primary_plane;
>   		primary->disable_plane = skylake_disable_primary_plane;
>   	} else if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_primary_formats = skl_primary_formats;
> -		num_formats = ARRAY_SIZE(skl_primary_formats);
> +		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> +			((pipe == PIPE_A || pipe == PIPE_B))) {
This will add support only for BXT & KBL, what about other platforms?
Please mention that as comment or update the check.

-Mahesh
> +			intel_primary_formats = nv12_primary_formats;
> +			num_formats = ARRAY_SIZE(nv12_primary_formats);
> +		} else {
> +			intel_primary_formats = skl_primary_formats;
> +			num_formats = ARRAY_SIZE(skl_primary_formats);
> +		}
>   		if (pipe < PIPE_C)
>   			modifiers = skl_format_modifiers_ccs;
>   		else
Kumar, Mahesh Oct. 10, 2017, 2:29 p.m. UTC | #2 
On Tuesday 10 October 2017 07:39 PM, Mahesh Kumar wrote:
> Hi,
>
>
> On Tuesday 10 October 2017 05:47 PM, Vidya Srinivas wrote:
>> From: Chandra Konduru <chandra.konduru@intel.com>
>>
>> This patch adds NV12 to list of supported formats for
>> primary plane
>>
>> v2: Rebased (Chandra Konduru)
>>
>> v3: Rebased (me)
>>
>> v4: Review comments by Ville addressed
>>     Removed the skl_primary_formats_with_nv12 and
>>     added NV12 case in existing skl_primary_formats
>>
>> v5: Rebased (me)
>>
>> v6: Missed the Tested-by/Reviewed-by in the previous series
>>     Adding the same to commit message in this version.
>>
>> v7: Review comments by Ville addressed
>>     Restricting the NV12 for BXT and on PIPE A and B
>>     Rebased (me)
>>
>> v8: Rebased (me)
>>     Modified restricting the NV12 support for both BXT and KBL.
>>
>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++--
>>   1 file changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index f71a704..e551f59 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -106,6 +106,22 @@
>>       DRM_FORMAT_MOD_INVALID
>>   };
>>   +static const uint32_t nv12_primary_formats[] = {
>> +    DRM_FORMAT_C8,
>> +    DRM_FORMAT_RGB565,
>> +    DRM_FORMAT_XRGB8888,
>> +    DRM_FORMAT_XBGR8888,
>> +    DRM_FORMAT_ARGB8888,
>> +    DRM_FORMAT_ABGR8888,
>> +    DRM_FORMAT_XRGB2101010,
>> +    DRM_FORMAT_XBGR2101010,
>> +    DRM_FORMAT_YUYV,
>> +    DRM_FORMAT_YVYU,
>> +    DRM_FORMAT_UYVY,
>> +    DRM_FORMAT_VYUY,
>> +    DRM_FORMAT_NV12,
>> +};
>> +
>>   /* Cursor formats */
>>   static const uint32_t intel_cursor_formats[] = {
>>       DRM_FORMAT_ARGB8888,
>> @@ -13256,8 +13272,14 @@ static bool 
>> intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
>>           primary->update_plane = skylake_update_primary_plane;
>>           primary->disable_plane = skylake_disable_primary_plane;
>>       } else if (INTEL_GEN(dev_priv) >= 9) {
>> -        intel_primary_formats = skl_primary_formats;
>> -        num_formats = ARRAY_SIZE(skl_primary_formats);
>> +        if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)) &&
>> +            ((pipe == PIPE_A || pipe == PIPE_B))) {
> This will add support only for BXT & KBL, what about other platforms?
> Please mention that as comment or update the check.
after digging more in comments, it seems matt is suggesting there could 
be customer with BXT stepping < E0, which doesn't support NV12 fully.
so IMO and as other suggested make is something like 
(!(IS_SKYLAKE(dev_priv) || IS_BXT_REVID(dev_priv, BXT_REVIC_A0, 
BXT_REVID_D0))
or others can comments if it should not be enabled for CFL/CNL yet?
>
> -Mahesh
>> +            intel_primary_formats = nv12_primary_formats;
>> +            num_formats = ARRAY_SIZE(nv12_primary_formats);
>> +        } else {
>> +            intel_primary_formats = skl_primary_formats;
>> +            num_formats = ARRAY_SIZE(skl_primary_formats);
>> +        }
>>           if (pipe < PIPE_C)
>>               modifiers = skl_format_modifiers_ccs;
>>           else
>
Srinivas, Vidya Oct. 10, 2017, 8:25 p.m. UTC | #3 
> -----Original Message-----

> From: Kumar, Mahesh1

> Sent: Tuesday, October 10, 2017 7:40 PM

> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-

> gfx@lists.freedesktop.org

> Cc: Shankar, Uma <uma.shankar@intel.com>; Konduru, Chandra

> <chandra.konduru@intel.com>; Kamath, Sunil <sunil.kamath@intel.com>;

> Maiti, Nabendu Bikash <nabendu.bikash.maiti@intel.com>

> Subject: Re: [PATCH 12/14] drm/i915: Add NV12 as supported format for

> primary plane

> 

> Hi,

> 

> 

> On Tuesday 10 October 2017 05:47 PM, Vidya Srinivas wrote:

> > From: Chandra Konduru <chandra.konduru@intel.com>

> >

> > This patch adds NV12 to list of supported formats for primary plane

> >

> > v2: Rebased (Chandra Konduru)

> >

> > v3: Rebased (me)

> >

> > v4: Review comments by Ville addressed

> > 	Removed the skl_primary_formats_with_nv12 and

> > 	added NV12 case in existing skl_primary_formats

> >

> > v5: Rebased (me)

> >

> > v6: Missed the Tested-by/Reviewed-by in the previous series

> > 	Adding the same to commit message in this version.

> >

> > v7: Review comments by Ville addressed

> > 	Restricting the NV12 for BXT and on PIPE A and B

> > 	Rebased (me)

> >

> > v8: Rebased (me)

> > 	Modified restricting the NV12 support for both BXT and KBL.

> >

> > Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>

> > Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>

> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>

> > Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>

> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>

> > ---

> >   drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++--

> >   1 file changed, 24 insertions(+), 2 deletions(-)

> >

> > diff --git a/drivers/gpu/drm/i915/intel_display.c

> > b/drivers/gpu/drm/i915/intel_display.c

> > index f71a704..e551f59 100644

> > --- a/drivers/gpu/drm/i915/intel_display.c

> > +++ b/drivers/gpu/drm/i915/intel_display.c

> > @@ -106,6 +106,22 @@

> >   	DRM_FORMAT_MOD_INVALID

> >   };

> >

> > +static const uint32_t nv12_primary_formats[] = {

> > +	DRM_FORMAT_C8,

> > +	DRM_FORMAT_RGB565,

> > +	DRM_FORMAT_XRGB8888,

> > +	DRM_FORMAT_XBGR8888,

> > +	DRM_FORMAT_ARGB8888,

> > +	DRM_FORMAT_ABGR8888,

> > +	DRM_FORMAT_XRGB2101010,

> > +	DRM_FORMAT_XBGR2101010,

> > +	DRM_FORMAT_YUYV,

> > +	DRM_FORMAT_YVYU,

> > +	DRM_FORMAT_UYVY,

> > +	DRM_FORMAT_VYUY,

> > +	DRM_FORMAT_NV12,

> > +};

> > +

> >   /* Cursor formats */

> >   static const uint32_t intel_cursor_formats[] = {

> >   	DRM_FORMAT_ARGB8888,

> > @@ -13256,8 +13272,14 @@ static bool

> intel_cursor_plane_format_mod_supported(struct drm_plane *plane,

> >   		primary->update_plane = skylake_update_primary_plane;

> >   		primary->disable_plane = skylake_disable_primary_plane;

> >   	} else if (INTEL_GEN(dev_priv) >= 9) {

> > -		intel_primary_formats = skl_primary_formats;

> > -		num_formats = ARRAY_SIZE(skl_primary_formats);

> > +		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)) &&

> > +			((pipe == PIPE_A || pipe == PIPE_B))) {

> This will add support only for BXT & KBL, what about other platforms?

> Please mention that as comment or update the check.


Thank you. I tested the series currently only on BXT APL and the patches were also tested by other teams on KBL.
That’s why did not add changes for other platforms yet. Was planning to add them as enhancement.

> 

> -Mahesh

> > +			intel_primary_formats = nv12_primary_formats;

> > +			num_formats =

> ARRAY_SIZE(nv12_primary_formats);

> > +		} else {

> > +			intel_primary_formats = skl_primary_formats;

> > +			num_formats = ARRAY_SIZE(skl_primary_formats);

> > +		}

> >   		if (pipe < PIPE_C)

> >   			modifiers = skl_format_modifiers_ccs;

> >   		else
Kristian Høgsberg Oct. 16, 2017, 10:33 p.m. UTC | #4 
Vidya Srinivas <vidya.srinivas@intel.com> writes:

> From: Chandra Konduru <chandra.konduru@intel.com>
>
> This patch adds NV12 to list of supported formats for
> primary plane
>
> v2: Rebased (Chandra Konduru)
>
> v3: Rebased (me)
>
> v4: Review comments by Ville addressed
> 	Removed the skl_primary_formats_with_nv12 and
> 	added NV12 case in existing skl_primary_formats
>
> v5: Rebased (me)
>
> v6: Missed the Tested-by/Reviewed-by in the previous series
> 	Adding the same to commit message in this version.
>
> v7: Review comments by Ville addressed
> 	Restricting the NV12 for BXT and on PIPE A and B
> 	Rebased (me)
>
> v8: Rebased (me)
> 	Modified restricting the NV12 support for both BXT and KBL.
>
> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f71a704..e551f59 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -106,6 +106,22 @@
>  	DRM_FORMAT_MOD_INVALID
>  };
>  
> +static const uint32_t nv12_primary_formats[] = {
> +	DRM_FORMAT_C8,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XRGB2101010,
> +	DRM_FORMAT_XBGR2101010,
> +	DRM_FORMAT_YUYV,
> +	DRM_FORMAT_YVYU,
> +	DRM_FORMAT_UYVY,
> +	DRM_FORMAT_VYUY,
> +	DRM_FORMAT_NV12,
> +};
> +

You need to extend skl_mod_supported() with a case for DRM_FORMAT_NV12
so that KMS can advertise the supported modifiers for NV12.

Kristian

>  /* Cursor formats */
>  static const uint32_t intel_cursor_formats[] = {
>  	DRM_FORMAT_ARGB8888,
> @@ -13256,8 +13272,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
>  		primary->update_plane = skylake_update_primary_plane;
>  		primary->disable_plane = skylake_disable_primary_plane;
>  	} else if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_primary_formats = skl_primary_formats;
> -		num_formats = ARRAY_SIZE(skl_primary_formats);
> +		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> +			((pipe == PIPE_A || pipe == PIPE_B))) {
> +			intel_primary_formats = nv12_primary_formats;
> +			num_formats = ARRAY_SIZE(nv12_primary_formats);
> +		} else {
> +			intel_primary_formats = skl_primary_formats;
> +			num_formats = ARRAY_SIZE(skl_primary_formats);
> +		}
>  		if (pipe < PIPE_C)
>  			modifiers = skl_format_modifiers_ccs;
>  		else
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f71a704..e551f59 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -106,6 +106,22 @@ 
 	DRM_FORMAT_MOD_INVALID
 };
 
+static const uint32_t nv12_primary_formats[] = {
+	DRM_FORMAT_C8,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+	DRM_FORMAT_NV12,
+};
+
 /* Cursor formats */
 static const uint32_t intel_cursor_formats[] = {
 	DRM_FORMAT_ARGB8888,
@@ -13256,8 +13272,14 @@  static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
 		primary->update_plane = skylake_update_primary_plane;
 		primary->disable_plane = skylake_disable_primary_plane;
 	} else if (INTEL_GEN(dev_priv) >= 9) {
-		intel_primary_formats = skl_primary_formats;
-		num_formats = ARRAY_SIZE(skl_primary_formats);
+		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)) &&
+			((pipe == PIPE_A || pipe == PIPE_B))) {
+			intel_primary_formats = nv12_primary_formats;
+			num_formats = ARRAY_SIZE(nv12_primary_formats);
+		} else {
+			intel_primary_formats = skl_primary_formats;
+			num_formats = ARRAY_SIZE(skl_primary_formats);
+		}
 		if (pipe < PIPE_C)
 			modifiers = skl_format_modifiers_ccs;
 		else