Re: [PATCH v3 08/14] drm/mediatek: Add DRM_MODE_ROTATE_0 to rotation property

["Shawn Sung (宋孝謙)" <Shawn.Sung@mediatek.com>]

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