From: Dmitry Osipenko <digetx@gmail.com>
To: Emil Velikov <emil.l.velikov@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>,
Thomas Zimmermann <tzimmermann@suse.de>,
Derek Basehore <dbasehore@chromium.org>,
Sam Ravnborg <sam@ravnborg.org>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Sean Paul <sean@poorly.run>,
linux-tegra@vger.kernel.org,
"Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>,
ML dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v2 0/5] 180 degrees rotation support for NVIDIA Tegra DRM
Date: Tue, 16 Jun 2020 20:20:57 +0300 [thread overview]
Message-ID: <646b3f37-0f72-7f3b-388f-f71dbcdd5c84@gmail.com> (raw)
In-Reply-To: <CACvgo50oSMbgXw1vHwVT4hhGe6g3YzKQEohCLJdfDq+0UaN1jw@mail.gmail.com>
16.06.2020 18:48, Emil Velikov пишет:
> On Tue, 16 Jun 2020 at 12:40, Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> 16.06.2020 01:26, Emil Velikov пишет:
>>> Hi Dmitry,
>>>
>>> On Mon, 15 Jun 2020 at 08:28, Dmitry Osipenko <digetx@gmail.com> wrote:
>>>>
>>>> Hello!
>>>>
>>>> This series adds 180° display plane rotation support to the NVIDIA Tegra
>>>> DRM driver which is needed for devices that have display panel physically
>>>> mounted upside-down, like Nexus 7 tablet device for example [1]. Since
>>>> DRM panel rotation is a new thing for a userspace, currently only
>>>> Opentegra Xorg driver handles the rotated display panel [2], but this
>>>> is good enough for the start.
>>>>
>>>> Note that later on it should be possible to implement a transparent 180°
>>>> display rotation for Tegra DRM driver which will remove the need to have
>>>> a bleeding edge userspace that knows how to rotate display planes and I'm
>>>> slowly working on it. For the starter we can go with the minimal rotation
>>>> support, so it's not a blocker.
>>>>
>>>> This series is based on the work that was made by Derek Basehore for the
>>>> Mediatek driver [3], his patch is included into this patchset. I added
>>>> my tested-by tag to the Derek's patch.
>>>>
>>>> Please review and apply, thanks in advance!
>>>>
>>>> [1] https://patchwork.ozlabs.org/project/linux-tegra/patch/20200607154327.18589-3-digetx@gmail.com/
>>>> [2] https://github.com/grate-driver/xf86-video-opentegra/commit/28eb20a3959bbe5bc3a3b67e55977093fd5114ca
>>>> [3] https://lkml.org/lkml/2020/3/5/1119
>>>>
>>>> Changelog:
>>>>
>>>> v2: - Dropped "drm/panel: Set display info in panel attach" patch, which
>>>> turned out to be obsolete now.
>>>>
>>>> - Renamed the cover-latter, hopefully this will fix the bouncing emails.
>>>>
>>>> Derek Basehore (1):
>>>> drm/panel: Add helper for reading DT rotation
>>>>
>>>> Dmitry Osipenko (4):
>>>> drm/panel: lvds: Set up panel orientation
>>>
>>> IMHO it's perfectly reasonable to report the panel orientation to
>>> userspace, which can apply plane rotation as needed.
>>>
>>> Although I see that this series, alike Derek's, has a couple of issues:
>>> - only a single panel driver is updated
>>> - rotation is _not_ listed as supported property, in said panel
>>> driver device-tree bindings
>>>
>>> My personal inclination is that we should aim for a comprehensive solution:
>>> - wire all panel drivers, as currently documented (quick grep list below)
>>> - document and wire-up the lvds and boe panels - as proposed by you
>>> and Derek respectively
>>>
>>> HTH
>>> Emil
>>>
>>> Documentation/devicetree/bindings/display/himax,hx8357d.txt:2
>>> Documentation/devicetree/bindings/display/ilitek,ili9225.txt:2
>>> Documentation/devicetree/bindings/display/ilitek,ili9341.txt:2
>>> Documentation/devicetree/bindings/display/ilitek,ili9486.yaml:2
>>> Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt:2
>>> Documentation/devicetree/bindings/display/panel/panel-common.yaml:2
>>> Documentation/devicetree/bindings/display/sitronix,st7586.txt:1
>>> Documentation/devicetree/bindings/display/sitronix,st7735r.yaml:2
>>
>> Rotation is a common DT panel property that is described in the
>> panel-common.yaml.
> The property was introduced almost exclusively for tiny drm panels.
> Those ones are a bit different from the rest (in panel/) -
> MIPI-DBI/SPI w/o (not connected at least) an actual GPU.
>
> To make it a bit better, the rotation is seemingly performed in the
> tiny driver itself ouch.
>
>> This property is supported by all panel bindings
>> because these bindings inherent the common properties from the
>> panel-common.yaml.
>>
> Seems like that was an unintentional change with the conversion to YAML.
> Beforehand only a few selected panels had rotation. Upon closer look -
> some panels do have follow-up fixes, to remove/limit the implicit
> inclusion.
Interesting.. my understanding that the rotation property is supposed to
be a generic property which represents physical orientation of a display
panel and hence it should be applicable to all panels.
> Sam seems like you've done most of the YAML conversion. IMHO it would
> make sense to revisit the patches and inherit common properties only
> as applicable.
>
>> I don't think that it makes sense to wire up rotation property to all
>> panel drivers at once because those drivers will be untested, at least I
>> don't know anything about those other panels and can't test them. It
>> will be much better to support the rotation on by as-needed basis for
>> each panel driver individually.
>
> How about CCing the author and reviewer asking them to test the patch?
> The only place where the patches might cause an issue is with tiny,
> although patches would still be appreciated.
There are quite a lot of panel drivers and I'm a bit doubtful that at
least half of devices that use those panels have any real use for the
rotation property. I could write the patches.. but in the end it could
be a wasted effort if nobody needs it, so I'd prefer not to do it.
next prev parent reply other threads:[~2020-06-16 17:21 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-14 20:01 [PATCH v2 0/5] 180 degrees rotation support for NVIDIA Tegra DRM Dmitry Osipenko
2020-06-14 20:01 ` [PATCH v2 1/5] drm/panel: Add helper for reading DT rotation Dmitry Osipenko
2020-06-14 20:01 ` [PATCH v2 2/5] drm/panel: lvds: Set up panel orientation Dmitry Osipenko
2020-06-14 20:01 ` [PATCH v2 3/5] drm/tegra: plane: Rename bottom_up to reflect_y Dmitry Osipenko
2020-06-14 20:01 ` [PATCH v2 4/5] drm/tegra: plane: Support horizontal reflection mode Dmitry Osipenko
2020-06-14 20:01 ` [PATCH v2 5/5] drm/tegra: plane: Support 180° rotation Dmitry Osipenko
2020-06-15 16:57 ` Ville Syrjälä
2020-06-15 18:07 ` Dmitry Osipenko
2020-06-15 21:47 ` Emil Velikov
2020-06-16 11:25 ` Dmitry Osipenko
2020-06-17 18:50 ` Dmitry Osipenko
2020-06-15 22:26 ` [PATCH v2 0/5] 180 degrees rotation support for NVIDIA Tegra DRM Emil Velikov
2020-06-16 11:40 ` Dmitry Osipenko
2020-06-16 15:48 ` Emil Velikov
2020-06-16 17:20 ` Dmitry Osipenko [this message]
2020-06-16 17:45 ` Laurent Pinchart
2020-06-16 21:25 ` Dmitry Osipenko
2020-06-16 18:54 ` Emil Velikov
2020-06-16 21:16 ` Dmitry Osipenko
2020-06-17 9:34 ` Daniel Stone
2020-06-17 16:27 ` Dmitry Osipenko
2020-06-16 12:00 ` Dmitry Osipenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=646b3f37-0f72-7f3b-388f-f71dbcdd5c84@gmail.com \
--to=digetx@gmail.com \
--cc=dbasehore@chromium.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=emil.l.velikov@gmail.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=sam@ravnborg.org \
--cc=sean@poorly.run \
--cc=thierry.reding@gmail.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).