1. Patchew
  2. linux
  3. [PATCH v5 00/13] Support IGT in display driver
  4. View patch

[PATCH v5 06/13] drm/mediatek: Turn off the layers with zero width or height

Hsiao Chien Sung posted 13 patches 9 months ago
There is a newer version of this series
[PATCH v5 06/13] drm/mediatek: Turn off the layers with zero width or height
Posted by Hsiao Chien Sung 9 months ago 
We found that IGT (Intel GPU Tool) will try to commit layers with
zero width or height and lead to undefined behaviors in hardware.
Disable the layers in such a situation.

Fixes: 777b7bc86a0a3 ("drm/mediatek: Add ovl_adaptor support for MT8195")
Fixes: fa97fe71f6f93 ("drm/mediatek: Add ETHDR support for MT8195")

Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 2 +-
 drivers/gpu/drm/mediatek/mtk_ethdr.c            | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
index d4a13a1402148..68a20312ac6f1 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
@@ -157,7 +157,7 @@ void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int idx,
 	merge = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MERGE0 + idx];
 	ethdr = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0];
 
-	if (!pending->enable) {
+	if (!pending->enable || !pending->width || !pending->height) {
 		mtk_merge_stop_cmdq(merge, cmdq_pkt);
 		mtk_mdp_rdma_stop(rdma_l, cmdq_pkt);
 		mtk_mdp_rdma_stop(rdma_r, cmdq_pkt);
diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c
index 73dc4da3ba3bd..69872b77922eb 100644
--- a/drivers/gpu/drm/mediatek/mtk_ethdr.c
+++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c
@@ -160,7 +160,12 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx,
 	if (idx >= 4)
 		return;
 
-	if (!pending->enable) {
+	if (!pending->enable || !pending->width || !pending->height) {
+		/*
+		 * instead of disabling layer with MIX_SRC_CON directly
+		 * set the size to 0 to avoid screen shift due to mixer
+		 * mode switch (hardware behavior)
+		 */
 		mtk_ddp_write(cmdq_pkt, 0, &mixer->cmdq_base, mixer->regs, MIX_L_SRC_SIZE(idx));
 		return;
 	}
-- 
2.18.0
Re: [PATCH v5 06/13] drm/mediatek: Turn off the layers with zero width or height
Posted by CK Hu (胡俊光) 8 months, 2 weeks ago 
Hi, Hsiao-chien:

On Thu, 2024-02-15 at 18:11 +0800, Hsiao Chien Sung wrote:
> We found that IGT (Intel GPU Tool) will try to commit layers with
> zero width or height and lead to undefined behaviors in hardware.
> Disable the layers in such a situation.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

I have reviewed ovl driver, ovl does not have this limitation, so it's
better to point out which hardware has this limitation. That's OK if
you have no information.

Regards,
CK

> 
> Fixes: 777b7bc86a0a3 ("drm/mediatek: Add ovl_adaptor support for
> MT8195")
> Fixes: fa97fe71f6f93 ("drm/mediatek: Add ETHDR support for MT8195")
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 2 +-
>  drivers/gpu/drm/mediatek/mtk_ethdr.c            | 7 ++++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> index d4a13a1402148..68a20312ac6f1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> @@ -157,7 +157,7 @@ void mtk_ovl_adaptor_layer_config(struct device
> *dev, unsigned int idx,
>  	merge = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MERGE0 +
> idx];
>  	ethdr = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0];
>  
> -	if (!pending->enable) {
> +	if (!pending->enable || !pending->width || !pending->height) {
>  		mtk_merge_stop_cmdq(merge, cmdq_pkt);
>  		mtk_mdp_rdma_stop(rdma_l, cmdq_pkt);
>  		mtk_mdp_rdma_stop(rdma_r, cmdq_pkt);
> diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c
> b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> index 73dc4da3ba3bd..69872b77922eb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c
> +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> @@ -160,7 +160,12 @@ void mtk_ethdr_layer_config(struct device *dev,
> unsigned int idx,
>  	if (idx >= 4)
>  		return;
>  
> -	if (!pending->enable) {
> +	if (!pending->enable || !pending->width || !pending->height) {
> +		/*
> +		 * instead of disabling layer with MIX_SRC_CON directly
> +		 * set the size to 0 to avoid screen shift due to mixer
> +		 * mode switch (hardware behavior)
> +		 */
>  		mtk_ddp_write(cmdq_pkt, 0, &mixer->cmdq_base, mixer-
> >regs, MIX_L_SRC_SIZE(idx));
>  		return;
>  	}
Re: [PATCH v5 06/13] drm/mediatek: Turn off the layers with zero width or height
Posted by Shawn Sung (宋孝謙) 7 months, 4 weeks ago 
Hi CK,

On Fri, 2024-03-01 at 07:51 +0000, CK Hu (胡俊光) wrote:
> Hi, Hsiao-chien:
> 
> On Thu, 2024-02-15 at 18:11 +0800, Hsiao Chien Sung wrote:
> > We found that IGT (Intel GPU Tool) will try to commit layers with
> > zero width or height and lead to undefined behaviors in hardware.
> > Disable the layers in such a situation.
> 
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> 
> I have reviewed ovl driver, ovl does not have this limitation, so
> it's
> better to point out which hardware has this limitation. That's OK if
> you have no information.
> 
Thank you for the confirmation. After checking with the designer, for
MT8195/MT8188, the height setting in OVL can be 0 but width cannot,
otherwise the hardware could hang. Although we are not sure if other
old platforms have the same behavior.

Regards,
Shawn
Re: [PATCH v5 06/13] drm/mediatek: Turn off the layers with zero width or height
Posted by AngeloGioacchino Del Regno 9 months ago 
Il 15/02/24 11:11, Hsiao Chien Sung ha scritto:
> We found that IGT (Intel GPU Tool) will try to commit layers with
> zero width or height and lead to undefined behaviors in hardware.
> Disable the layers in such a situation.
> 
> Fixes: 777b7bc86a0a3 ("drm/mediatek: Add ovl_adaptor support for MT8195")
> Fixes: fa97fe71f6f93 ("drm/mediatek: Add ETHDR support for MT8195")
> 
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>

This commit should be sent separately from this series, as it is fixing things
that are not related just to IGT, but also to corner cases in regular non-testing
usecases.

In any case, it's not mandatory as that depends on what the maintainer prefers,
so it's CK's call anyway.

Besides that,

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 2 +-
>   drivers/gpu/drm/mediatek/mtk_ethdr.c            | 7 ++++++-
>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> index d4a13a1402148..68a20312ac6f1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> @@ -157,7 +157,7 @@ void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int idx,
>   	merge = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MERGE0 + idx];
>   	ethdr = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0];
>   
> -	if (!pending->enable) {
> +	if (!pending->enable || !pending->width || !pending->height) {
>   		mtk_merge_stop_cmdq(merge, cmdq_pkt);
>   		mtk_mdp_rdma_stop(rdma_l, cmdq_pkt);
>   		mtk_mdp_rdma_stop(rdma_r, cmdq_pkt);
> diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> index 73dc4da3ba3bd..69872b77922eb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_ethdr.c
> +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> @@ -160,7 +160,12 @@ void mtk_ethdr_layer_config(struct device *dev, unsigned int idx,
>   	if (idx >= 4)
>   		return;
>   
> -	if (!pending->enable) {
> +	if (!pending->enable || !pending->width || !pending->height) {
> +		/*
> +		 * instead of disabling layer with MIX_SRC_CON directly
> +		 * set the size to 0 to avoid screen shift due to mixer
> +		 * mode switch (hardware behavior)
> +		 */
>   		mtk_ddp_write(cmdq_pkt, 0, &mixer->cmdq_base, mixer->regs, MIX_L_SRC_SIZE(idx));
>   		return;
>   	}
Re: [PATCH v5 06/13] drm/mediatek: Turn off the layers with zero width or height
Posted by Shawn Sung (宋孝謙) 9 months ago 
On Thu, 2024-02-15 at 11:45 +0100, AngeloGioacchino Del Regno wrote:
> Il 15/02/24 11:11, Hsiao Chien Sung ha scritto:
> > We found that IGT (Intel GPU Tool) will try to commit layers with
> > zero width or height and lead to undefined behaviors in hardware.
> > Disable the layers in such a situation.
> > 
> > Fixes: 777b7bc86a0a3 ("drm/mediatek: Add ovl_adaptor support for
> > MT8195")
> > Fixes: fa97fe71f6f93 ("drm/mediatek: Add ETHDR support for MT8195")
> > 
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> 
> This commit should be sent separately from this series, as it is
> fixing things
> that are not related just to IGT, but also to corner cases in regular
> non-testing
> usecases.
> 
> In any case, it's not mandatory as that depends on what the
> maintainer prefers,
> so it's CK's call anyway.
> 
> Besides that,
> 
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>

Got it. Will discuss this with CK.

This bug is found when running IGT test while one of the test item
commits a layer with zero width and cuase the device to freeze.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.