drm: atmel-hlcdc: fix atmel_xlcdc_plane_setup_scaler()

[PATCH] drm: atmel-hlcdc: fix atmel_xlcdc_plane_setup_scaler()

Posted by Manikandan Muralidharan 1 month, 1 week ago

From: Cyrille Pitchen <cyrille.pitchen@microchip.com>

On SoCs, like the SAM9X75, which embed the XLCDC ip, the registers that
configure the unified scaling engine were not filled with proper values.

Indeed, for YCbCr formats, the VXSCFACT bitfield of the HEOCFG25
register and the HXSCFACT bitfield of the HEOCFG27 register were
incorrect.

For 4:2:0 formats, both vertical and horizontal factors for
chroma chanels should be divided by 2 from the factors for the luma
channel. Hence:

HEOCFG24.VXSYFACT = VFACTOR
HEOCFG25.VSXCFACT = VFACTOR / 2
HEOCFG26.HXSYFACT = HFACTOR
HEOCFG27.HXSCFACT = HFACTOR / 2

However, for 4:2:2 formats, only the horizontal factor for chroma
chanels should be divided by 2 from the factor for the luma channel;
the vertical factor is the same for all the luma and chroma channels.
Hence:

HEOCFG24.VXSYFACT = VFACTOR
HEOCFG25.VXSCFACT = VFACTOR
HEOCFG26.HXSYFACT = HFACTOR
HEOCFG27.HXSCFACT = HFACTOR / 2

Fixes: d498771b0b83 ("drm: atmel_hlcdc: Add support for XLCDC using IP specific driver ops")
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
---
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 27 ++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 4bcaf2cd7672..41c7351ae811 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -365,13 +365,34 @@ void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
 				    xfactor);
 
 	/*
-	 * With YCbCr 4:2:2 and YCbYcr 4:2:0 window resampling, configuration
-	 * register LCDC_HEOCFG25.VXSCFACT and LCDC_HEOCFG27.HXSCFACT is half
+	 * With YCbCr 4:2:0 window resampling, configuration register
+	 * LCDC_HEOCFG25.VXSCFACT and LCDC_HEOCFG27.HXSCFACT values are half
 	 * the value of yfactor and xfactor.
+	 *
+	 * On the other hand, with YCbCr 4:2:2 window resampling, only the
+	 * configuration register LCDC_HEOCFG27.HXSCFACT value is half the value
+	 * of the xfactor; the value of LCDC_HEOCFG25.VXSCFACT is yfactor (no
+	 * division by 2).
 	 */
-	if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
+	switch (state->base.fb->format->format) {
+	/* YCbCr 4:2:2 */
+	case DRM_FORMAT_YUYV:
+	case DRM_FORMAT_UYVY:
+	case DRM_FORMAT_YVYU:
+	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_YUV422:
+	case DRM_FORMAT_NV61:
+		xfactor /= 2;
+		break;
+
+	/* YCbCr 4:2:0 */
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_NV21:
 		yfactor /= 2;
 		xfactor /= 2;
+		break;
+	default:
+		break;
 	}
 
 	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
-- 
2.25.1

Re: [PATCH] drm: atmel-hlcdc: fix atmel_xlcdc_plane_setup_scaler()

Posted by Nicolas Ferre 1 month ago

On 14/10/2024 at 11:49, Manikandan Muralidharan wrote:
> From: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> 
> On SoCs, like the SAM9X75, which embed the XLCDC ip, the registers that
> configure the unified scaling engine were not filled with proper values.
> 
> Indeed, for YCbCr formats, the VXSCFACT bitfield of the HEOCFG25
> register and the HXSCFACT bitfield of the HEOCFG27 register were
> incorrect.
> 
> For 4:2:0 formats, both vertical and horizontal factors for
> chroma chanels should be divided by 2 from the factors for the luma
> channel. Hence:
> 
> HEOCFG24.VXSYFACT = VFACTOR
> HEOCFG25.VSXCFACT = VFACTOR / 2
> HEOCFG26.HXSYFACT = HFACTOR
> HEOCFG27.HXSCFACT = HFACTOR / 2
> 
> However, for 4:2:2 formats, only the horizontal factor for chroma
> chanels should be divided by 2 from the factor for the luma channel;
> the vertical factor is the same for all the luma and chroma channels.
> Hence:
> 
> HEOCFG24.VXSYFACT = VFACTOR
> HEOCFG25.VXSCFACT = VFACTOR
> HEOCFG26.HXSYFACT = HFACTOR
> HEOCFG27.HXSCFACT = HFACTOR / 2
> 
> Fixes: d498771b0b83 ("drm: atmel_hlcdc: Add support for XLCDC using IP specific driver ops")
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>

Thanks Mani and Cyrille:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Best regards,
  Nicolas

> ---
>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 27 ++++++++++++++++---
>   1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 4bcaf2cd7672..41c7351ae811 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -365,13 +365,34 @@ void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>   				    xfactor);
>   
>   	/*
> -	 * With YCbCr 4:2:2 and YCbYcr 4:2:0 window resampling, configuration
> -	 * register LCDC_HEOCFG25.VXSCFACT and LCDC_HEOCFG27.HXSCFACT is half
> +	 * With YCbCr 4:2:0 window resampling, configuration register
> +	 * LCDC_HEOCFG25.VXSCFACT and LCDC_HEOCFG27.HXSCFACT values are half
>   	 * the value of yfactor and xfactor.
> +	 *
> +	 * On the other hand, with YCbCr 4:2:2 window resampling, only the
> +	 * configuration register LCDC_HEOCFG27.HXSCFACT value is half the value
> +	 * of the xfactor; the value of LCDC_HEOCFG25.VXSCFACT is yfactor (no
> +	 * division by 2).
>   	 */
> -	if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
> +	switch (state->base.fb->format->format) {
> +	/* YCbCr 4:2:2 */
> +	case DRM_FORMAT_YUYV:
> +	case DRM_FORMAT_UYVY:
> +	case DRM_FORMAT_YVYU:
> +	case DRM_FORMAT_VYUY:
> +	case DRM_FORMAT_YUV422:
> +	case DRM_FORMAT_NV61:
> +		xfactor /= 2;
> +		break;
> +
> +	/* YCbCr 4:2:0 */
> +	case DRM_FORMAT_YUV420:
> +	case DRM_FORMAT_NV21:
>   		yfactor /= 2;
>   		xfactor /= 2;
> +		break;
> +	default:
> +		break;
>   	}
>   
>   	atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,

Re: [PATCH] drm: atmel-hlcdc: fix atmel_xlcdc_plane_setup_scaler()

Posted by Dmitry Baryshkov 1 month, 1 week ago

On Mon, Oct 14, 2024 at 03:19:42PM +0530, Manikandan Muralidharan wrote:
> From: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> 
> On SoCs, like the SAM9X75, which embed the XLCDC ip, the registers that
> configure the unified scaling engine were not filled with proper values.
> 
> Indeed, for YCbCr formats, the VXSCFACT bitfield of the HEOCFG25
> register and the HXSCFACT bitfield of the HEOCFG27 register were
> incorrect.
> 
> For 4:2:0 formats, both vertical and horizontal factors for
> chroma chanels should be divided by 2 from the factors for the luma
> channel. Hence:
> 
> HEOCFG24.VXSYFACT = VFACTOR
> HEOCFG25.VSXCFACT = VFACTOR / 2
> HEOCFG26.HXSYFACT = HFACTOR
> HEOCFG27.HXSCFACT = HFACTOR / 2
> 
> However, for 4:2:2 formats, only the horizontal factor for chroma
> chanels should be divided by 2 from the factor for the luma channel;
> the vertical factor is the same for all the luma and chroma channels.
> Hence:
> 
> HEOCFG24.VXSYFACT = VFACTOR
> HEOCFG25.VXSCFACT = VFACTOR
> HEOCFG26.HXSYFACT = HFACTOR
> HEOCFG27.HXSCFACT = HFACTOR / 2
> 
> Fixes: d498771b0b83 ("drm: atmel_hlcdc: Add support for XLCDC using IP specific driver ops")
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 27 ++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry