| Message ID | 20240215101119.12629-7-shawn.sung@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support IGT in display driver | expand |
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; > }
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.
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; > }
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
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; }
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(-)