Message-ID: <fa443308-7610-9060-68eb-e14e446dd4bf@gmail.com>
Date:   Mon, 18 May 2020 10:36:12 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Sean Paul <sean@...rly.run>
Cc:     "dbasehore ." <dbasehore@...omium.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Sam Ravnborg <sam@...nborg.org>,
        David Airlie <airlied@...ux.ie>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH v10 0/2] Panel rotation patches

12.05.2020 23:59, Sean Paul пишет:
> On Thu, Apr 16, 2020 at 7:03 PM Dmitry Osipenko <digetx@...il.com> wrote:
>>
>> 15.04.2020 00:32, dbasehore . пишет:
>>> On Tue, Apr 14, 2020 at 2:18 PM Dmitry Osipenko <digetx@...il.com> wrote:
>>>>
>>>> 14.04.2020 22:32, dbasehore . пишет:
>>>>> Hi Dmitry, sorry for the late reply.
>>>>>
>>>>> On Sun, Mar 8, 2020 at 12:25 PM Dmitry Osipenko <digetx@...il.com> wrote:
>>>>>>
>>>>>> 06.03.2020 03:21, Derek Basehore пишет:
>>>>>>> This adds the plumbing for reading panel rotation from the devicetree
>>>>>>> and sets up adding a panel property for the panel orientation on
>>>>>>> Mediatek SoCs when a rotation is present.
>>>>>>
>>>>>> Hello Derek and everyone,
>>>>>>
>>>>>> I'm looking at adding display rotation support to NVIDIA Tegra DRM
>>>>>> driver because some devices have display panel physically mounted
>>>>>> upside-down, and thus, display controller's scan-out needs to be rotated
>>>>>> by 180° in this case.
>>>>>>
>>>>>> Derek, yours panel-rotation patches add support for assigning panel's
>>>>>> orientation to the connector, but then only primary display plane
>>>>>> receives rotation value in [1], while rotation needs to be applied to
>>>>>> all available overlay/cursor planes and this should happen in other
>>>>>> places than [1] as well.
>>>>>
>>>>> This is intended. We don't correct the output in the kernel. We
>>>>> instead rely on notifying userspace that the panel is rotated, then we
>>>>> handle it there.
>>>>>
>>>>>>
>>>>>> [1] drm_client_modeset_commit_atomic()
>>>>>>
>>>>>> Please also note that in a case of the scan-out rotation, plane's
>>>>>> coordinates need to be changed in accordance to the display's rotation.
>>>>>>
>>>>>> I looked briefly through the DRM code and my understanding that the DRM
>>>>>> core currently doesn't support use-case where scan-out needs to rotated
>>>>>> based on a panel's orientation, correct? Is it the use-case you're
>>>>>> working on for the Mediatek driver?
>>>>>
>>>>> Yes, we rely on userspace to rotate the output. The major reason for
>>>>> this is because there may not be a "free" hardware rotation that can
>>>>> be applied to the overlay. Sean Paul and others also preferred that
>>>>> userspace control what is output to the screen instead of the kernel
>>>>> taking care of it. This code just adds the drm property to the panel.
>>>>>
>>>>
>>>> Could you please explain what that userspace is?
>>>
>>> This was added for Chrome OS, which uses its own graphics stack,
>>> Ozone, instead of Xorg.
>>>
>>
>> Thank you very much for the clarification.
>>
>> It's probably not a big problem for something monolithic and customized
>> like ChromeOS to issue a software update in order to take into account
>> all specifics of a particular device, but this doesn't work nicely for a
>> generic software, like a usual Linux distro.
>>
>>>> AFAIK, things like Xorg modesetting don't support that orientation property.
>>
>> In my case it's not only the display panel which is upside-down, but
>> also the touchscreen. Hence both display output and touchscreen input
>> need to be rotated at once, otherwise you'll end up with either display
>> or input being upside-down.
>>
>> The 180° rotation should be free on NVIDIA Tegra. There are no known
>> limitations for the planes and BSP kernel video driver handles the
>> plane's coordinates/framebuffer rotation within the driver.
>>
>> The kernel's input subsystem allows us to transparently (for userspace)
>> remap the touchscreen input (by specifying generic touchscreen
>> device-tree properties), while this is not the case for the DRM subsystem.
>>
>> @Thierry, @Sean, @Daniel, could you please help me to understand how a
>> coordinated display / input rotation could be implemented, making the
>> rotation transparent to the user (i.e. avoiding xorg.conf hacking and
>> etc)? It should be nice if display's output could be flipped within the
>> DRM driver, hiding this fact from userspace.
> 
> I think the right thing to do is to fix userspace to respect this
> property, since that has the most communal benefit.

Hello Sean,

This will be ideal, but it's difficult to achieve in a loosely
controlled userspace environment.

> However(!!) if you don't want to do that, how about inspecting the
> info->panel_orientation value after drm_panel_attach in tegra driver
> and then adjusting rotation values in the driver. Of course, you
> wouldn't want to expose the panel orientation property since you don't
> want userspaces to be double-rotating on you, but it's optional so
> you'd be fine.

Thank you very much for the suggestion, I'll be trying it out soon.

>>
>> Will it be okay if we'll add a transparent-rotation support specifically
>> to the Tegra DRM driver? For example if device-tree contains
>> nvidia,display-flip-y property, then the Tegra DRM driver will take care
>> of rotating coordinates/framebuffer of the display planes.
> 
> I don't think this is necessary, but it also wouldn't really be
> appropriate to put software attributes into devicetree anyways.

Yes, I'm also not feeling very excited about this variant.

Powered by blists - more mailing lists