[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZA9xKlScy9/LS753@ravnborg.org>
Date: Mon, 13 Mar 2023 19:53:30 +0100
From: Sam Ravnborg <sam@...nborg.org>
To: Neil Armstrong <neil.armstrong@...aro.org>
Cc: Jianhua Lu <lujianhua000@...il.com>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Thierry Reding <thierry.reding@...il.com>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Linus Walleij <linus.walleij@...aro.org>,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org
Subject: Re: [PATCH v5 2/2] drm/panel: Add driver for Novatek NT36523
On Mon, Mar 13, 2023 at 09:06:50AM +0100, Neil Armstrong wrote:
> On 11/03/2023 13:46, Jianhua Lu wrote:
> > On Sat, Mar 11, 2023 at 01:38:52PM +0100, Konrad Dybcio wrote:
> > >
> > >
> > > On 11.03.2023 13:32, Jianhua Lu wrote:
> > > > Add a driver for panels using the Novatek NT36523 display driver IC.
> > > >
> > > > Signed-off-by: Jianhua Lu <lujianhua000@...il.com>
> > > > ---
> > > [...]
> > >
> > > > +
> > > > +static int nt36523_get_modes(struct drm_panel *panel,
> > > > + struct drm_connector *connector)
> > > > +{
> > > > + struct panel_info *pinfo = to_panel_info(panel);
> > > > + int i;
> > > > +
> > > > + for (i = 0; i < pinfo->desc->num_modes; i++) {
> > > > + const struct drm_display_mode *m = &pinfo->desc->modes[i];
> > > > + struct drm_display_mode *mode;
> > > > +
> > > > + mode = drm_mode_duplicate(connector->dev, m);
> > > > + if (!mode) {
> > > > + dev_err(panel->dev, "failed to add mode %ux%u@%u\n",
> > > > + m->hdisplay, m->vdisplay, drm_mode_vrefresh(m));
> > > > + return -ENOMEM;
> > > > + }
> > > > +
> > > > + mode->type = DRM_MODE_TYPE_DRIVER;
> > > > + if (pinfo->desc->num_modes == 1)
> > > > + mode->type |= DRM_MODE_TYPE_PREFERRED;
> > > That's not quite correct, as that means "if you have more than one
> > > defined panel mode (say 60Hz and 120 Hz), there will be no preferred one".
> > This piece of code I see in the other panels, so I'm not sure if it is
> > correct.
Jianhua is correct that the same code exists in several places,
and from a quick browse I consider all the cases bogus.
It would be fine if someone volunteered to fix all the panels so we
avoid this bug to creep into more panel drivers.
Sam
Powered by blists - more mailing lists