[PATCH v4 16/17] drm/mediatek: Fix errors when reporting rotation capability

Hsiao Chien Sung posted 17 patches 11 months ago
There is a newer version of this series
[PATCH v4 16/17] drm/mediatek: Fix errors when reporting rotation capability
Posted by Hsiao Chien Sung 11 months ago
Create rotation property according to the hardware capability.
Since currently OVL of all chips support same rotation,
no need to define it in the driver data.

Fixes: 84d805753983 ("drm/mediatek: Support reflect-y plane rotation")

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_drv.h        |  1 +
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c        | 18 ++++++------------
 .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c    |  9 +++++++++
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c    |  1 +
 drivers/gpu/drm/mediatek/mtk_drm_plane.c       |  2 +-
 5 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index 4d6e8b667bc3..c5afeb7c5527 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -127,6 +127,7 @@ void mtk_ovl_adaptor_register_vblank_cb(struct device *dev, void (*vblank_cb)(vo
 void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev);
 void mtk_ovl_adaptor_enable_vblank(struct device *dev);
 void mtk_ovl_adaptor_disable_vblank(struct device *dev);
+unsigned int mtk_ovl_adaptor_supported_rotations(struct device *dev);
 void mtk_ovl_adaptor_start(struct device *dev);
 void mtk_ovl_adaptor_stop(struct device *dev);
 unsigned int mtk_ovl_adaptor_layer_nr(struct device *dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 78749dabeb43..f019737078f6 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -399,6 +399,10 @@ unsigned int mtk_ovl_layer_nr(struct device *dev)
 
 unsigned int mtk_ovl_supported_rotations(struct device *dev)
 {
+	/*
+	 * although currently OVL can only do reflection,
+	 * reflect x + reflect y = rotate 180
+	 */
 	return DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180 |
 	       DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y;
 }
@@ -407,27 +411,17 @@ int mtk_ovl_layer_check(struct device *dev, unsigned int idx,
 			struct mtk_plane_state *mtk_state)
 {
 	struct drm_plane_state *state = &mtk_state->base;
-	unsigned int rotation = 0;
 
-	rotation = drm_rotation_simplify(state->rotation,
-					 DRM_MODE_ROTATE_0 |
-					 DRM_MODE_REFLECT_X |
-					 DRM_MODE_REFLECT_Y);
-	rotation &= ~DRM_MODE_ROTATE_0;
-
-	/* We can only do reflection, not rotation */
-	if ((rotation & DRM_MODE_ROTATE_MASK) != 0)
+	if (state->rotation & ~mtk_ovl_supported_rotations(dev))
 		return -EINVAL;
 
 	/*
 	 * TODO: Rotating/reflecting YUV buffers is not supported at this time.
 	 *	 Only RGB[AX] variants are supported.
 	 */
-	if (state->fb->format->is_yuv && rotation != 0)
+	if (state->fb->format->is_yuv && (state->rotation & ~DRM_MODE_ROTATE_0))
 		return -EINVAL;
 
-	state->rotation = rotation;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
index 4398db9a6276..273c79d37bef 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
@@ -383,6 +383,15 @@ void mtk_ovl_adaptor_register_vblank_cb(struct device *dev, void (*vblank_cb)(vo
 				     vblank_cb, vblank_cb_data);
 }
 
+unsigned int mtk_ovl_adaptor_supported_rotations(struct device *dev)
+{
+	/*
+	 * should still return DRM_MODE_ROTATE_0 if rotation is not supported,
+	 * or IGT will fail.
+	 */
+	return DRM_MODE_ROTATE_0;
+}
+
 void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev)
 {
 	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index ffa4868b1222..206dd6f6f99e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -422,6 +422,7 @@ static const struct mtk_ddp_comp_funcs ddp_ovl_adaptor = {
 	.remove = mtk_ovl_adaptor_remove_comp,
 	.get_formats = mtk_ovl_adaptor_get_formats,
 	.get_num_formats = mtk_ovl_adaptor_get_num_formats,
+	.supported_rotations = mtk_ovl_adaptor_supported_rotations,
 };
 
 static const char * const mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] = {
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index ff300426b590..e73b9793dee2 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -343,7 +343,7 @@ int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane,
 		return err;
 	}
 
-	if (supported_rotations & ~DRM_MODE_ROTATE_0) {
+	if (supported_rotations) {
 		err = drm_plane_create_rotation_property(plane,
 							 DRM_MODE_ROTATE_0,
 							 supported_rotations);
-- 
2.18.0
Re: [PATCH v4 16/17] drm/mediatek: Fix errors when reporting rotation capability
Posted by CK Hu (胡俊光) 10 months, 2 weeks ago
Hi, Hsiao-chien:

On Tue, 2023-12-12 at 20:19 +0800, Hsiao Chien Sung wrote:
> Create rotation property according to the hardware capability.
> Since currently OVL of all chips support same rotation,
> no need to define it in the driver data.
> 
> Fixes: 84d805753983 ("drm/mediatek: Support reflect-y plane
> rotation")
> 
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h        |  1 +
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c        | 18 ++++++--------
> ----
>  .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c    |  9 +++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c    |  1 +
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c       |  2 +-
>  5 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 4d6e8b667bc3..c5afeb7c5527 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -127,6 +127,7 @@ void mtk_ovl_adaptor_register_vblank_cb(struct
> device *dev, void (*vblank_cb)(vo
>  void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev);
>  void mtk_ovl_adaptor_enable_vblank(struct device *dev);
>  void mtk_ovl_adaptor_disable_vblank(struct device *dev);
> +unsigned int mtk_ovl_adaptor_supported_rotations(struct device
> *dev);
>  void mtk_ovl_adaptor_start(struct device *dev);
>  void mtk_ovl_adaptor_stop(struct device *dev);
>  unsigned int mtk_ovl_adaptor_layer_nr(struct device *dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 78749dabeb43..f019737078f6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -399,6 +399,10 @@ unsigned int mtk_ovl_layer_nr(struct device
> *dev)
>  
>  unsigned int mtk_ovl_supported_rotations(struct device *dev)
>  {
> +	/*
> +	 * although currently OVL can only do reflection,
> +	 * reflect x + reflect y = rotate 180
> +	 */
>  	return DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180 |
>  	       DRM_MODE_REFLECT_X | DRM_MODE_REFLECT_Y;
>  }
> @@ -407,27 +411,17 @@ int mtk_ovl_layer_check(struct device *dev,
> unsigned int idx,
>  			struct mtk_plane_state *mtk_state)
>  {
>  	struct drm_plane_state *state = &mtk_state->base;
> -	unsigned int rotation = 0;
>  
> -	rotation = drm_rotation_simplify(state->rotation,
> -					 DRM_MODE_ROTATE_0 |
> -					 DRM_MODE_REFLECT_X |
> -					 DRM_MODE_REFLECT_Y);
> -	rotation &= ~DRM_MODE_ROTATE_0;
> -
> -	/* We can only do reflection, not rotation */
> -	if ((rotation & DRM_MODE_ROTATE_MASK) != 0)
> +	if (state->rotation & ~mtk_ovl_supported_rotations(dev))
>  		return -EINVAL;
>  
>  	/*
>  	 * TODO: Rotating/reflecting YUV buffers is not supported at
> this time.
>  	 *	 Only RGB[AX] variants are supported.
>  	 */
> -	if (state->fb->format->is_yuv && rotation != 0)
> +	if (state->fb->format->is_yuv && (state->rotation &
> ~DRM_MODE_ROTATE_0))

You still no explain what you do here.

>  		return -EINVAL;
>  
> -	state->rotation = rotation;
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> index 4398db9a6276..273c79d37bef 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> @@ -383,6 +383,15 @@ void mtk_ovl_adaptor_register_vblank_cb(struct
> device *dev, void (*vblank_cb)(vo
>  				     vblank_cb, vblank_cb_data);
>  }
>  
> +unsigned int mtk_ovl_adaptor_supported_rotations(struct device *dev)
> +{
> +	/*
> +	 * should still return DRM_MODE_ROTATE_0 if rotation is not
> supported,
> +	 * or IGT will fail.
> +	 */
> +	return DRM_MODE_ROTATE_0;
> +}
> +
>  void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev)
>  {
>  	struct mtk_disp_ovl_adaptor *ovl_adaptor =
> dev_get_drvdata(dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index ffa4868b1222..206dd6f6f99e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -422,6 +422,7 @@ static const struct mtk_ddp_comp_funcs
> ddp_ovl_adaptor = {
>  	.remove = mtk_ovl_adaptor_remove_comp,
>  	.get_formats = mtk_ovl_adaptor_get_formats,
>  	.get_num_formats = mtk_ovl_adaptor_get_num_formats,
> +	.supported_rotations = mtk_ovl_adaptor_supported_rotations,
>  };
>  
>  static const char * const mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] =
> {
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index ff300426b590..e73b9793dee2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -343,7 +343,7 @@ int mtk_plane_init(struct drm_device *dev, struct
> drm_plane *plane,
>  		return err;
>  	}
>  
> -	if (supported_rotations & ~DRM_MODE_ROTATE_0) {
> +	if (supported_rotations) {

Try report issue to IGT team.

Regards,
CK

>  		err = drm_plane_create_rotation_property(plane,
>  							 DRM_MODE_ROTAT
> E_0,
>  							 supported_rota
> tions);