| Message ID | 20240620-igt-v3-8-a9d62d2e2c7e@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | This series fixes the errors of MediaTek display driver found by IGT. | expand |
Hi, On Wed, Jun 19, 2024 at 9:39 AM Hsiao Chien Sung via B4 Relay <devnull+shawn.sung.mediatek.com@kernel.org> wrote: > > From: Hsiao Chien Sung <shawn.sung@mediatek.com> > > Always add DRM_MODE_ROTATE_0 to rotation property to meet > IGT's (Intel GPU Tools) requirement. > > Reviewed-by: CK Hu <ck.hu@mediatek.com> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.") > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 6 +++++- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 17 +++++------------ > drivers/gpu/drm/mediatek/mtk_plane.c | 2 +- > 3 files changed, 11 insertions(+), 14 deletions(-) FWIW, this patch got into ChromeOS's 5.15 branch via stable merge and apparently broke things. As a short term fix we've reverted it there: https://crrev.com/c/5960799 ...apparently the patch is fine on newer kernels so maybe there is a missing dependency? Hopefully someone on this list can dig into this and either post the revert to stable 5.15 kernels or suggest additional backports. -Doug
Hi Doug, On Thu, 2024-10-24 at 13:47 -0700, Doug Anderson wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > Hi, > > On Wed, Jun 19, 2024 at 9:39 AM Hsiao Chien Sung via B4 Relay > <devnull+shawn.sung.mediatek.com@kernel.org> wrote: > > > > From: Hsiao Chien Sung <shawn.sung@mediatek.com> > > > > Always add DRM_MODE_ROTATE_0 to rotation property to meet > > IGT's (Intel GPU Tools) requirement. > > > > Reviewed-by: CK Hu <ck.hu@mediatek.com> > > Reviewed-by: AngeloGioacchino Del Regno < > angelogioacchino.delregno@collabora.com> > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC > MT8173.") > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > > --- > > drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 6 +++++- > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 17 +++++------------ > > drivers/gpu/drm/mediatek/mtk_plane.c | 2 +- > > 3 files changed, 11 insertions(+), 14 deletions(-) > > FWIW, this patch got into ChromeOS's 5.15 branch via stable merge and > apparently broke things. As a short term fix we've reverted it there: > > https://crrev.com/c/5960799 Thank you for reporting this issue. We are currently investigating the bug. Since I am unable to access the Google issue tracker [1], could you please provide more details about this bug? The message in the revert commit mentions "hana/sycamore360" (MT8173) and it appears that there is a rotation issue in tablet mode. > > ...apparently the patch is fine on newer kernels so maybe there is a > missing dependency? Hopefully someone on this list can dig into this > and either post the revert to stable 5.15 kernels or suggest > additional backports. > There are known issues [2] regarding forward compatibility. Could you confirm whether this patch is unable to resolve the mentioned problem? Thanks. [1] https://issuetracker.google.com/issues/369688659 [2] https://patchwork.kernel.org/project/linux-mediatek/list/?series=896964 > > -Doug Best regards, Shawn
Hi Shawn, On Thu, Oct 24, 2024 at 6:32 PM Shawn Sung (宋孝謙) <Shawn.Sung@mediatek.com> wrote: > > Hi Doug, > > On Thu, 2024-10-24 at 13:47 -0700, Doug Anderson wrote: > > > > External email : Please do not click links or open attachments until > > you have verified the sender or the content. > > Hi, > > > > On Wed, Jun 19, 2024 at 9:39 AM Hsiao Chien Sung via B4 Relay > > <devnull+shawn.sung.mediatek.com@kernel.org> wrote: > > > > > > From: Hsiao Chien Sung <shawn.sung@mediatek.com> > > > > > > Always add DRM_MODE_ROTATE_0 to rotation property to meet > > > IGT's (Intel GPU Tools) requirement. > > > > > > Reviewed-by: CK Hu <ck.hu@mediatek.com> > > > Reviewed-by: AngeloGioacchino Del Regno < > > angelogioacchino.delregno@collabora.com> > > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC > > MT8173.") > > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > > > --- > > > drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 6 +++++- > > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 17 +++++------------ > > > drivers/gpu/drm/mediatek/mtk_plane.c | 2 +- > > > 3 files changed, 11 insertions(+), 14 deletions(-) > > > > FWIW, this patch got into ChromeOS's 5.15 branch via stable merge and > > apparently broke things. As a short term fix we've reverted it there: > > > > https://crrev.com/c/5960799 > > Thank you for reporting this issue. We are currently investigating the > bug. > > Since I am unable to access the Google issue tracker [1], could you > please provide more details about this bug? The message in the revert > commit mentions "hana/sycamore360" (MT8173) and it appears that there > is a rotation issue in tablet mode. Thanks for the followup. I've only been peripherally involved in the problem, but I can at least copy the relevant bits over. It looks as if the problem is somehow only showing up when running Android apps on those devices, so whatever the problem is it's subtle. The report says that the apps work OK when the device is in tablet mode and in one rotation but the problem shows up when rotated 90 degrees. The report says that "Screen content appears inverted". To me it also sounds _possible_ that the problem is somewhere in our userspace. I think Hsin-Yi and Ross are continuing to dig a bit more. Maybe once they've dug they can add any details they find or can loop in others as it makes sense? > > ...apparently the patch is fine on newer kernels so maybe there is a > > missing dependency? Hopefully someone on this list can dig into this > > and either post the revert to stable 5.15 kernels or suggest > > additional backports. > > > > There are known issues [2] regarding forward compatibility. Could you > confirm whether this patch is unable to resolve the mentioned problem? > Thanks. > > [1] https://issuetracker.google.com/issues/369688659 > [2] > https://patchwork.kernel.org/project/linux-mediatek/list/?series=896964 The patches in [2] look related to alpha blending but I think they are seeing issues related to rotation. ...so I'm going to assume it's different? I don't have this hardware in front of me, so I'm just going by the report. -Doug
Hi Doug, On Fri, 2024-10-25 at 09:35 -0700, Doug Anderson wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > Hi Shawn, > > On Thu, Oct 24, 2024 at 6:32 PM Shawn Sung (宋孝謙) > <Shawn.Sung@mediatek.com> wrote: > > > > Hi Doug, > > > > On Thu, 2024-10-24 at 13:47 -0700, Doug Anderson wrote: > > > > > > External email : Please do not click links or open attachments > > > until > > > you have verified the sender or the content. > > > Hi, > > > > > > On Wed, Jun 19, 2024 at 9:39 AM Hsiao Chien Sung via B4 Relay > > > <devnull+shawn.sung.mediatek.com@kernel.org> wrote: > > > > > > > > From: Hsiao Chien Sung <shawn.sung@mediatek.com> > > > > > > > > Always add DRM_MODE_ROTATE_0 to rotation property to meet > > > > IGT's (Intel GPU Tools) requirement. > > > > > > > > Reviewed-by: CK Hu <ck.hu@mediatek.com> > > > > Reviewed-by: AngeloGioacchino Del Regno < > > > > > > angelogioacchino.delregno@collabora.com> > > > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek > > > > SoC > > > > > > MT8173.") > > > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > > > > --- > > > > drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 6 +++++- > > > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 17 +++++------------ > > > > drivers/gpu/drm/mediatek/mtk_plane.c | 2 +- > > > > 3 files changed, 11 insertions(+), 14 deletions(-) > > > > > > FWIW, this patch got into ChromeOS's 5.15 branch via stable merge > > > and > > > apparently broke things. As a short term fix we've reverted it > > > there: > > > > > > https://urldefense.com/v3/__https://crrev.com/c/5960799__;!!CTRNKA9wMg0ARbw!kSI2lyZJ5F8SCpAFfKCeZsL1qie2qRvpXWPhtD4IlHik7uS4q6hdR6EjPbskQxlTNP6OIBSwPOKU2KtFf5NXKQ$ > > > > Thank you for reporting this issue. We are currently investigating > > the > > bug. > > > > Since I am unable to access the Google issue tracker [1], could you > > please provide more details about this bug? The message in the > > revert > > commit mentions "hana/sycamore360" (MT8173) and it appears that > > there > > is a rotation issue in tablet mode. > > Thanks for the followup. I've only been peripherally involved in the > problem, but I can at least copy the relevant bits over. > > It looks as if the problem is somehow only showing up when running > Android apps on those devices, so whatever the problem is it's > subtle. > The report says that the apps work OK when the device is in tablet > mode and in one rotation but the problem shows up when rotated 90 > degrees. The report says that "Screen content appears inverted". To > me > it also sounds _possible_ that the problem is somewhere in our > userspace. Thank you for providing the details. We have also reached out to our partner at Google and gained a better understanding of the situation. We discovered that the capability for 180-degree rotation was not previously claimed. However, we reported this capability by adding DRM_MODE_ROTATE_180 to the plane property, as combining flip-x and flip-y effectively results in a 180-degree rotation. Unfortunately, it appears that we did not properly handle the rotation property in the driver, which has led to the current issues. The reason there is no problem after reverting this patch is likely because, when the driver does not support rotation, Android apps will handle screen rotation via software. After this patch, since we claim that our driver supports 180-degree rotation, the app attempts to utilize hardware for this function, which has resulted in the bug. > > I think Hsin-Yi and Ross are continuing to dig a bit more. Maybe once > they've dug they can add any details they find or can loop in others > as it makes sense? Thank you for your assistance, and we will continue to investigate this matter. Since I am no longer involved in the related project, Jason-JH will assist in investigating this issue and will submit a fix once we confirm the root cause. > > > > > ...apparently the patch is fine on newer kernels so maybe there > > > is a > > > missing dependency? Hopefully someone on this list can dig into > > > this > > > and either post the revert to stable 5.15 kernels or suggest > > > additional backports. > > > > > > > There are known issues [2] regarding forward compatibility. Could > > you > > confirm whether this patch is unable to resolve the mentioned > > problem? > > Thanks. > > > > [1] > > https://urldefense.com/v3/__https://issuetracker.google.com/issues/369688659__;!!CTRNKA9wMg0ARbw!kSI2lyZJ5F8SCpAFfKCeZsL1qie2qRvpXWPhtD4IlHik7uS4q6hdR6EjPbskQxlTNP6OIBSwPOKU2KsGQwdobA$ > > [2] > > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/list/?series=896964__;!!CTRNKA9wMg0ARbw!kSI2lyZJ5F8SCpAFfKCeZsL1qie2qRvpXWPhtD4IlHik7uS4q6hdR6EjPbskQxlTNP6OIBSwPOKU2KuXChWBjA$ > > The patches in [2] look related to alpha blending but I think they > are > seeing issues related to rotation. ...so I'm going to assume it's > different? I don't have this hardware in front of me, so I'm just > going by the report. No problem. Based on your detailed description above, it seems we have identified the possible cause. > > -Doug Best regards, Shawn
diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h index 26236691ce4c..f7fe2e08dc8e 100644 --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h @@ -192,7 +192,11 @@ unsigned int mtk_ddp_comp_supported_rotations(struct mtk_ddp_comp *comp) if (comp->funcs && comp->funcs->supported_rotations) return comp->funcs->supported_rotations(comp->dev); - return 0; + /* + * In order to pass IGT tests, DRM_MODE_ROTATE_0 is required when + * rotation is not supported. + */ + return DRM_MODE_ROTATE_0; } static inline unsigned int mtk_ddp_comp_layer_nr(struct mtk_ddp_comp *comp) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 693560fa34e8..26b598b9f71f 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -305,27 +305,20 @@ 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) + /* check if any unsupported rotation is set */ + 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. + * Since DRM_MODE_ROTATE_0 means "no rotation", we should not + * reject layers with this property. */ - 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_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c index a74b26d35985..1723d4333f37 100644 --- a/drivers/gpu/drm/mediatek/mtk_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_plane.c @@ -338,7 +338,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);