diff mbox series

Add support for Color encoding YCBCR_BT2020

Message ID 20200214105316.16076-1-kishore.kadiyala@intel.com (mailing list archive)
State New, archived
Headers show
Series Add support for Color encoding YCBCR_BT2020 | expand

Commit Message

Kishore Kadiyala Feb. 14, 2020, 10:53 a.m. UTC
Currently the plane property doesn't have support for YCBCR_BT2020,
which enables the corresponding color conversion mode on plane CSC.

Signed-off-by: Kishore Kadiyala <kishore.kadiyala@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_sprite.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Jani Nikula Feb. 14, 2020, 11:25 a.m. UTC | #1
On Fri, 14 Feb 2020, Kishore Kadiyala <kishore.kadiyala@intel.com> wrote:
> Currently the plane property doesn't have support for YCBCR_BT2020,
> which enables the corresponding color conversion mode on plane CSC.

Please prefix your patch subjects properly. "drm/i915: " at a minimum
for stuff touching i915.

No need to resend for this, but for future reference.

BR,
Jani.


>
> Signed-off-by: Kishore Kadiyala <kishore.kadiyala@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_sprite.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 7abeefe8dce5..5169a7260d7c 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -3011,6 +3011,7 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
>  	struct intel_plane *plane;
>  	enum drm_plane_type plane_type;
>  	unsigned int supported_rotations;
> +	unsigned int supported_csc;
>  	unsigned int possible_crtcs;
>  	const u64 *modifiers;
>  	const u32 *formats;
> @@ -3088,9 +3089,13 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
>  					   DRM_MODE_ROTATE_0,
>  					   supported_rotations);
>  
> +	supported_csc = BIT(DRM_COLOR_YCBCR_BT601) | BIT(DRM_COLOR_YCBCR_BT709);
> +
> +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> +		supported_csc |= BIT(DRM_COLOR_YCBCR_BT2020);
> +
>  	drm_plane_create_color_properties(&plane->base,
> -					  BIT(DRM_COLOR_YCBCR_BT601) |
> -					  BIT(DRM_COLOR_YCBCR_BT709),
> +					  supported_csc,
>  					  BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
>  					  BIT(DRM_COLOR_YCBCR_FULL_RANGE),
>  					  DRM_COLOR_YCBCR_BT709,
Ville Syrjala Feb. 14, 2020, 1:10 p.m. UTC | #2
On Fri, Feb 14, 2020 at 04:23:16PM +0530, Kishore Kadiyala wrote:
> Currently the plane property doesn't have support for YCBCR_BT2020,
> which enables the corresponding color conversion mode on plane CSC.
> 
> Signed-off-by: Kishore Kadiyala <kishore.kadiyala@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_sprite.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 7abeefe8dce5..5169a7260d7c 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -3011,6 +3011,7 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
>  	struct intel_plane *plane;
>  	enum drm_plane_type plane_type;
>  	unsigned int supported_rotations;
> +	unsigned int supported_csc;
>  	unsigned int possible_crtcs;
>  	const u64 *modifiers;
>  	const u32 *formats;
> @@ -3088,9 +3089,13 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
>  					   DRM_MODE_ROTATE_0,
>  					   supported_rotations);
>  
> +	supported_csc = BIT(DRM_COLOR_YCBCR_BT601) | BIT(DRM_COLOR_YCBCR_BT709);
> +
> +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> +		supported_csc |= BIT(DRM_COLOR_YCBCR_BT2020);

Missing the actual code to enable it on glk and icl+ sdr planes, so
this will not work.

IIRC there was also some kind of hw fail related to some BT.something
conversions. But I can't remember if that was in the RGB->RGB or
YUV->RGB logic.

Also clearly commit 27ee72477c9b ("drm/i915/icl: Handle YCbCr to RGB
conversion for BT2020 case") was never actually tested :(

> +
>  	drm_plane_create_color_properties(&plane->base,
> -					  BIT(DRM_COLOR_YCBCR_BT601) |
> -					  BIT(DRM_COLOR_YCBCR_BT709),
> +					  supported_csc,
>  					  BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
>  					  BIT(DRM_COLOR_YCBCR_FULL_RANGE),
>  					  DRM_COLOR_YCBCR_BT709,
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Shankar, Uma Feb. 14, 2020, 2:27 p.m. UTC | #3
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Friday, February 14, 2020 6:40 PM
> To: Kadiyala, Kishore <kishore.kadiyala@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [Intel-gfx] [PATCH] Add support for Color encoding YCBCR_BT2020
> 
> On Fri, Feb 14, 2020 at 04:23:16PM +0530, Kishore Kadiyala wrote:
> > Currently the plane property doesn't have support for YCBCR_BT2020,
> > which enables the corresponding color conversion mode on plane CSC.
> >
> > Signed-off-by: Kishore Kadiyala <kishore.kadiyala@intel.com>
> > Cc: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_sprite.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index 7abeefe8dce5..5169a7260d7c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -3011,6 +3011,7 @@ skl_universal_plane_create(struct drm_i915_private
> *dev_priv,
> >  	struct intel_plane *plane;
> >  	enum drm_plane_type plane_type;
> >  	unsigned int supported_rotations;
> > +	unsigned int supported_csc;
> >  	unsigned int possible_crtcs;
> >  	const u64 *modifiers;
> >  	const u32 *formats;
> > @@ -3088,9 +3089,13 @@ skl_universal_plane_create(struct drm_i915_private
> *dev_priv,
> >  					   DRM_MODE_ROTATE_0,
> >  					   supported_rotations);
> >
> > +	supported_csc = BIT(DRM_COLOR_YCBCR_BT601) |
> > +BIT(DRM_COLOR_YCBCR_BT709);
> > +
> > +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > +		supported_csc |= BIT(DRM_COLOR_YCBCR_BT2020);
> 
> Missing the actual code to enable it on glk and icl+ sdr planes, so this will not work.

Yeah this need to be handled for Non HDR planes.

> IIRC there was also some kind of hw fail related to some BT.something conversions.
> But I can't remember if that was in the RGB->RGB or
> YUV->RGB logic.

Will try to check this out. What kind of fails Ville, was it not all working or artifacts ?
 
> Also clearly commit 27ee72477c9b ("drm/i915/icl: Handle YCbCr to RGB conversion
> for BT2020 case") was never actually tested :(

Yeah, this seem to miss the BT2020 export from driver. IGT has the stuff to test this.
Will work with Kishore to fix this.

Regards,
Uma Shankar
> 
> > +
> >  	drm_plane_create_color_properties(&plane->base,
> > -					  BIT(DRM_COLOR_YCBCR_BT601) |
> > -					  BIT(DRM_COLOR_YCBCR_BT709),
> > +					  supported_csc,
> >
> BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
> >  					  BIT(DRM_COLOR_YCBCR_FULL_RANGE),
> >  					  DRM_COLOR_YCBCR_BT709,
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel
Ville Syrjala Feb. 14, 2020, 2:32 p.m. UTC | #4
On Fri, Feb 14, 2020 at 02:27:35PM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Friday, February 14, 2020 6:40 PM
> > To: Kadiyala, Kishore <kishore.kadiyala@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>
> > Subject: Re: [Intel-gfx] [PATCH] Add support for Color encoding YCBCR_BT2020
> > 
> > On Fri, Feb 14, 2020 at 04:23:16PM +0530, Kishore Kadiyala wrote:
> > > Currently the plane property doesn't have support for YCBCR_BT2020,
> > > which enables the corresponding color conversion mode on plane CSC.
> > >
> > > Signed-off-by: Kishore Kadiyala <kishore.kadiyala@intel.com>
> > > Cc: Uma Shankar <uma.shankar@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_sprite.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > index 7abeefe8dce5..5169a7260d7c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > @@ -3011,6 +3011,7 @@ skl_universal_plane_create(struct drm_i915_private
> > *dev_priv,
> > >  	struct intel_plane *plane;
> > >  	enum drm_plane_type plane_type;
> > >  	unsigned int supported_rotations;
> > > +	unsigned int supported_csc;
> > >  	unsigned int possible_crtcs;
> > >  	const u64 *modifiers;
> > >  	const u32 *formats;
> > > @@ -3088,9 +3089,13 @@ skl_universal_plane_create(struct drm_i915_private
> > *dev_priv,
> > >  					   DRM_MODE_ROTATE_0,
> > >  					   supported_rotations);
> > >
> > > +	supported_csc = BIT(DRM_COLOR_YCBCR_BT601) |
> > > +BIT(DRM_COLOR_YCBCR_BT709);
> > > +
> > > +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > > +		supported_csc |= BIT(DRM_COLOR_YCBCR_BT2020);
> > 
> > Missing the actual code to enable it on glk and icl+ sdr planes, so this will not work.
> 
> Yeah this need to be handled for Non HDR planes.
> 
> > IIRC there was also some kind of hw fail related to some BT.something conversions.
> > But I can't remember if that was in the RGB->RGB or
> > YUV->RGB logic.
> 
> Will try to check this out. What kind of fails Ville, was it not all working or artifacts ?

IIRC a wrong coefficient was used in one of the hardcoded hw
matrices. Should be listed on the bspec w/a page.
Shankar, Uma Feb. 14, 2020, 2:57 p.m. UTC | #5
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Friday, February 14, 2020 8:03 PM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: Kadiyala, Kishore <kishore.kadiyala@intel.com>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] Add support for Color encoding YCBCR_BT2020
> 
> On Fri, Feb 14, 2020 at 02:27:35PM +0000, Shankar, Uma wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Sent: Friday, February 14, 2020 6:40 PM
> > > To: Kadiyala, Kishore <kishore.kadiyala@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma
> > > <uma.shankar@intel.com>
> > > Subject: Re: [Intel-gfx] [PATCH] Add support for Color encoding
> > > YCBCR_BT2020
> > >
> > > On Fri, Feb 14, 2020 at 04:23:16PM +0530, Kishore Kadiyala wrote:
> > > > Currently the plane property doesn't have support for
> > > > YCBCR_BT2020, which enables the corresponding color conversion mode on
> plane CSC.
> > > >
> > > > Signed-off-by: Kishore Kadiyala <kishore.kadiyala@intel.com>
> > > > Cc: Uma Shankar <uma.shankar@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_sprite.c | 9 +++++++--
> > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > index 7abeefe8dce5..5169a7260d7c 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > @@ -3011,6 +3011,7 @@ skl_universal_plane_create(struct
> > > > drm_i915_private
> > > *dev_priv,
> > > >  	struct intel_plane *plane;
> > > >  	enum drm_plane_type plane_type;
> > > >  	unsigned int supported_rotations;
> > > > +	unsigned int supported_csc;
> > > >  	unsigned int possible_crtcs;
> > > >  	const u64 *modifiers;
> > > >  	const u32 *formats;
> > > > @@ -3088,9 +3089,13 @@ skl_universal_plane_create(struct
> > > > drm_i915_private
> > > *dev_priv,
> > > >  					   DRM_MODE_ROTATE_0,
> > > >  					   supported_rotations);
> > > >
> > > > +	supported_csc = BIT(DRM_COLOR_YCBCR_BT601) |
> > > > +BIT(DRM_COLOR_YCBCR_BT709);
> > > > +
> > > > +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > > > +		supported_csc |= BIT(DRM_COLOR_YCBCR_BT2020);
> > >
> > > Missing the actual code to enable it on glk and icl+ sdr planes, so this will not
> work.
> >
> > Yeah this need to be handled for Non HDR planes.
> >
> > > IIRC there was also some kind of hw fail related to some BT.something
> conversions.
> > > But I can't remember if that was in the RGB->RGB or
> > > YUV->RGB logic.
> >
> > Will try to check this out. What kind of fails Ville, was it not all working or artifacts
> ?
> 
> IIRC a wrong coefficient was used in one of the hardcoded hw matrices. Should be
> listed on the bspec w/a page.

Able to retrieve that, looks like it's for RGB709 to RGB2020 conversion on SDR planes.
So recommendation is not to enable this conversion for SDR planes.

Thanks Ville.

Regards,
Uma Shankar

> --
> Ville Syrjälä
> Intel
Kishore Kadiyala Feb. 17, 2020, 3:09 a.m. UTC | #6
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Friday, February 14, 2020 4:56 PM
> To: Kadiyala, Kishore <kishore.kadiyala@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Kadiyala, Kishore <kishore.kadiyala@intel.com>
> Subject: Re: [Intel-gfx] [PATCH] Add support for Color encoding YCBCR_BT2020
> 
> On Fri, 14 Feb 2020, Kishore Kadiyala <kishore.kadiyala@intel.com> wrote:
> > Currently the plane property doesn't have support for YCBCR_BT2020,
> > which enables the corresponding color conversion mode on plane CSC.
> 
> Please prefix your patch subjects properly. "drm/i915: " at a minimum for stuff
> touching i915.

   Sure Jani, I'll include prefix from next time.

> 
> No need to resend for this, but for future reference.
  Ok 

Regards,
Kishore
> 
> BR,
> Jani.
> 
> 
> >
> > Signed-off-by: Kishore Kadiyala <kishore.kadiyala@intel.com>
> > Cc: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_sprite.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index 7abeefe8dce5..5169a7260d7c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -3011,6 +3011,7 @@ skl_universal_plane_create(struct drm_i915_private
> *dev_priv,
> >  	struct intel_plane *plane;
> >  	enum drm_plane_type plane_type;
> >  	unsigned int supported_rotations;
> > +	unsigned int supported_csc;
> >  	unsigned int possible_crtcs;
> >  	const u64 *modifiers;
> >  	const u32 *formats;
> > @@ -3088,9 +3089,13 @@ skl_universal_plane_create(struct
> drm_i915_private *dev_priv,
> >  					   DRM_MODE_ROTATE_0,
> >  					   supported_rotations);
> >
> > +	supported_csc = BIT(DRM_COLOR_YCBCR_BT601) |
> > +BIT(DRM_COLOR_YCBCR_BT709);
> > +
> > +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > +		supported_csc |= BIT(DRM_COLOR_YCBCR_BT2020);
> > +
> >  	drm_plane_create_color_properties(&plane->base,
> > -					  BIT(DRM_COLOR_YCBCR_BT601) |
> > -					  BIT(DRM_COLOR_YCBCR_BT709),
> > +					  supported_csc,
> >
> BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
> >
> BIT(DRM_COLOR_YCBCR_FULL_RANGE),
> >  					  DRM_COLOR_YCBCR_BT709,
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 7abeefe8dce5..5169a7260d7c 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -3011,6 +3011,7 @@  skl_universal_plane_create(struct drm_i915_private *dev_priv,
 	struct intel_plane *plane;
 	enum drm_plane_type plane_type;
 	unsigned int supported_rotations;
+	unsigned int supported_csc;
 	unsigned int possible_crtcs;
 	const u64 *modifiers;
 	const u32 *formats;
@@ -3088,9 +3089,13 @@  skl_universal_plane_create(struct drm_i915_private *dev_priv,
 					   DRM_MODE_ROTATE_0,
 					   supported_rotations);
 
+	supported_csc = BIT(DRM_COLOR_YCBCR_BT601) | BIT(DRM_COLOR_YCBCR_BT709);
+
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+		supported_csc |= BIT(DRM_COLOR_YCBCR_BT2020);
+
 	drm_plane_create_color_properties(&plane->base,
-					  BIT(DRM_COLOR_YCBCR_BT601) |
-					  BIT(DRM_COLOR_YCBCR_BT709),
+					  supported_csc,
 					  BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
 					  BIT(DRM_COLOR_YCBCR_FULL_RANGE),
 					  DRM_COLOR_YCBCR_BT709,