Re: [Intel-gfx] [v3] drm/i915/icl: Handle YCbCr to RGB conversion for BT2020 case

2019-05-09 Thread Shankar, Uma 


>-Original Message-
>From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
>Sent: Thursday, May 9, 2019 10:02 PM
>To: Shankar, Uma 
>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville ; 
>Lankhorst,
>Maarten 
>Subject: Re: [Intel-gfx] [v3] drm/i915/icl: Handle YCbCr to RGB conversion for 
>BT2020
>case
>
>On Thu, May 09, 2019 at 09:57:19PM +0530, Uma Shankar wrote:
>> Currently input csc for YCbCR to RGB conversion handles only
>> BT601 and Bt709. Extending it to support BT2020 as well.
>>
>> v2: Fixed the co-efficients for LR to FR conversion, as suggested by
>> Ville.
>>
>> v3: Fixed Y Pre-offset in case of Full Range YCbCr as suggested by
>> Ville.
>>
>> Signed-off-by: Uma Shankar 
>> Signed-off-by: Shashank Sharma 
>> ---
>>  drivers/gpu/drm/i915/intel_sprite.c | 31
>> +--
>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>> b/drivers/gpu/drm/i915/intel_sprite.c
>> index 2913e89..c9c970f 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -433,6 +433,18 @@ int intel_plane_check_src_coordinates(struct
>intel_plane_state *plane_state)
>>  0x9EF8, 0x7800, 0xABF8,
>>  0x0, 0x7800,  0x7ED8,
>>  },
>> +/*
>> + * BT.2020 full range YCbCr -> full range RGB
>> + * The matrix required is :
>> + * [1.000, 0.000, 1.474,
>> + *  1.000, -0.1645, -0.5713,
>> + *  1.000, 1.8814, 0.]
>> + */
>> +[DRM_COLOR_YCBCR_BT2020] = {
>> +0x7BC8, 0x7800, 0x0,
>> +0x8928, 0x7800, 0xAA88,
>> +0x0, 0x7800, 0x7F10,
>> +},
>>  };
>>
>>  /* Matrix for Limited Range to Full Range Conversion */ @@ -461,6
>> +473,18 @@ int intel_plane_check_src_coordinates(struct intel_plane_state
>*plane_state)
>>  0x, 0x7918, 0xADA8,
>>  0x0, 0x7918,  0x6870,
>>  },
>> +/*
>> + * BT.2020 Limited range YCbCr -> full range RGB
>> + * The matrix required is :
>> + * [1.164, 0.000, 1.678,
>> + *  1.164, -0.1873, -0.6504,
>> + *  1.164, 2.1417, 0.]
>> + */
>> +[DRM_COLOR_YCBCR_BT2020] = {
>> +0x7D70, 0x7950, 0x0,
>> +0x8A68, 0x7950, 0xAC00,
>> +0x0, 0x7950, 0x6890,
>> +},
>>  };
>>  const u16 *csc;
>>
>> @@ -481,8 +505,11 @@ int intel_plane_check_src_coordinates(struct
>> intel_plane_state *plane_state)
>>
>>  I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 0),
>>PREOFF_YUV_TO_RGB_HI);
>> -I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
>> -  PREOFF_YUV_TO_RGB_ME);
>> +if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
>> +I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1), 0);
>> +else
>> +I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
>> +  PREOFF_YUV_TO_RGB_ME);
>
>I think this could probably be a separate patch since it affects
>BT.601/BT.709 as well. Oh, and the matrix coefficients for 601/709 seem 
>similarly off
>as what you had in this patch originally. So I think we want yet another patch 
>to fix
>those up. Just a bit surprised that even the current igts pass with those 
>coefficients.

Sure, will split the same and resend. I agree BT601/709 has similar issue and 
was planning to
send a fixup patch for the same. I will investigate what is going on with IGT 
and how its
working fine.

>Anyways,
>Reviewed-by: Ville Syrjälä  (you can keep this 
>on both
>patch if you split).

Thanks Ville.

>PS. pls. cc me @linux.intel.com rather than @intel.com. I generally ignore 
>patches
>going to the @intel.com address. Not that I really care one way or the other 
>whether
>a patch was cc:d to me anyway.

Sure, will add your linux.intel.com while cc'ing you :)

Regards,
Uma Shankar

>
>>  I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 2),
>>PREOFF_YUV_TO_RGB_LO);
>>  I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 0), 0x0);
>> --
>> 1.9.1
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [v3] drm/i915/icl: Handle YCbCr to RGB conversion for BT2020 case

2019-05-09 Thread Ville Syrjälä 
On Thu, May 09, 2019 at 09:57:19PM +0530, Uma Shankar wrote:
> Currently input csc for YCbCR to RGB conversion handles only
> BT601 and Bt709. Extending it to support BT2020 as well.
> 
> v2: Fixed the co-efficients for LR to FR conversion,
> as suggested by Ville.
> 
> v3: Fixed Y Pre-offset in case of Full Range YCbCr as suggested
> by Ville.
> 
> Signed-off-by: Uma Shankar 
> Signed-off-by: Shashank Sharma 
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 31 +--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 2913e89..c9c970f 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -433,6 +433,18 @@ int intel_plane_check_src_coordinates(struct 
> intel_plane_state *plane_state)
>   0x9EF8, 0x7800, 0xABF8,
>   0x0, 0x7800,  0x7ED8,
>   },
> + /*
> +  * BT.2020 full range YCbCr -> full range RGB
> +  * The matrix required is :
> +  * [1.000, 0.000, 1.474,
> +  *  1.000, -0.1645, -0.5713,
> +  *  1.000, 1.8814, 0.]
> +  */
> + [DRM_COLOR_YCBCR_BT2020] = {
> + 0x7BC8, 0x7800, 0x0,
> + 0x8928, 0x7800, 0xAA88,
> + 0x0, 0x7800, 0x7F10,
> + },
>   };
>  
>   /* Matrix for Limited Range to Full Range Conversion */
> @@ -461,6 +473,18 @@ int intel_plane_check_src_coordinates(struct 
> intel_plane_state *plane_state)
>   0x, 0x7918, 0xADA8,
>   0x0, 0x7918,  0x6870,
>   },
> + /*
> +  * BT.2020 Limited range YCbCr -> full range RGB
> +  * The matrix required is :
> +  * [1.164, 0.000, 1.678,
> +  *  1.164, -0.1873, -0.6504,
> +  *  1.164, 2.1417, 0.]
> +  */
> + [DRM_COLOR_YCBCR_BT2020] = {
> + 0x7D70, 0x7950, 0x0,
> + 0x8A68, 0x7950, 0xAC00,
> + 0x0, 0x7950, 0x6890,
> + },
>   };
>   const u16 *csc;
>  
> @@ -481,8 +505,11 @@ int intel_plane_check_src_coordinates(struct 
> intel_plane_state *plane_state)
>  
>   I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 0),
> PREOFF_YUV_TO_RGB_HI);
> - I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
> -   PREOFF_YUV_TO_RGB_ME);
> + if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> + I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1), 0);
> + else
> + I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
> +   PREOFF_YUV_TO_RGB_ME);

I think this could probably be a separate patch since it affects
BT.601/BT.709 as well. Oh, and the matrix coefficients for 601/709 seem
similarly off as what you had in this patch originally. So I think we
want yet another patch to fix those up. Just a bit surprised that even
the current igts pass with those coefficients.

Anyways,
Reviewed-by: Ville Syrjälä 
(you can keep this on both patch if you split).

PS. pls. cc me @linux.intel.com rather than @intel.com. I generally 
ignore patches going to the @intel.com address. Not that I really
care one way or the other whether a patch was cc:d to me anyway.

>   I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 2),
> PREOFF_YUV_TO_RGB_LO);
>   I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 0), 0x0);
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [v3] drm/i915/icl: Handle YCbCr to RGB conversion for BT2020 case

2019-05-09 Thread Uma Shankar 
Currently input csc for YCbCR to RGB conversion handles only
BT601 and Bt709. Extending it to support BT2020 as well.

v2: Fixed the co-efficients for LR to FR conversion,
as suggested by Ville.

v3: Fixed Y Pre-offset in case of Full Range YCbCr as suggested
by Ville.

Signed-off-by: Uma Shankar 
Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/i915/intel_sprite.c | 31 +--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 2913e89..c9c970f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -433,6 +433,18 @@ int intel_plane_check_src_coordinates(struct 
intel_plane_state *plane_state)
0x9EF8, 0x7800, 0xABF8,
0x0, 0x7800,  0x7ED8,
},
+   /*
+* BT.2020 full range YCbCr -> full range RGB
+* The matrix required is :
+* [1.000, 0.000, 1.474,
+*  1.000, -0.1645, -0.5713,
+*  1.000, 1.8814, 0.]
+*/
+   [DRM_COLOR_YCBCR_BT2020] = {
+   0x7BC8, 0x7800, 0x0,
+   0x8928, 0x7800, 0xAA88,
+   0x0, 0x7800, 0x7F10,
+   },
};
 
/* Matrix for Limited Range to Full Range Conversion */
@@ -461,6 +473,18 @@ int intel_plane_check_src_coordinates(struct 
intel_plane_state *plane_state)
0x, 0x7918, 0xADA8,
0x0, 0x7918,  0x6870,
},
+   /*
+* BT.2020 Limited range YCbCr -> full range RGB
+* The matrix required is :
+* [1.164, 0.000, 1.678,
+*  1.164, -0.1873, -0.6504,
+*  1.164, 2.1417, 0.]
+*/
+   [DRM_COLOR_YCBCR_BT2020] = {
+   0x7D70, 0x7950, 0x0,
+   0x8A68, 0x7950, 0xAC00,
+   0x0, 0x7950, 0x6890,
+   },
};
const u16 *csc;
 
@@ -481,8 +505,11 @@ int intel_plane_check_src_coordinates(struct 
intel_plane_state *plane_state)
 
I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 0),
  PREOFF_YUV_TO_RGB_HI);
-   I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
- PREOFF_YUV_TO_RGB_ME);
+   if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
+   I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1), 0);
+   else
+   I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 1),
+ PREOFF_YUV_TO_RGB_ME);
I915_WRITE_FW(PLANE_INPUT_CSC_PREOFF(pipe, plane_id, 2),
  PREOFF_YUV_TO_RGB_LO);
I915_WRITE_FW(PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 0), 0x0);
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx