diff mbox series

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

Message ID 20240215101119.12629-7-shawn.sung@mediatek.com
State New
Headers
Series Support IGT in display driver |

Commit Message

Shawn Sung (宋孝謙) Feb. 15, 2024, 10:11 a.m. UTC 
  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(-)

Comments

AngeloGioacchino Del Regno Feb. 15, 2024, 10:45 a.m. UTC | #1 
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;
>   	}
Shawn Sung (宋孝謙) Feb. 16, 2024, 2:03 a.m. UTC | #2 
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.
CK Hu (胡俊光) March 1, 2024, 7:51 a.m. UTC | #3 
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;
>  	}
diff mbox series

Patch

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;
 	}