lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 28 Jun 2022 07:52:31 -0700 From: Doug Anderson <dianders@...omium.org> To: Rex Nie <rexnie3@...il.com> Cc: Linux ARM <linux-arm-kernel@...ts.infradead.org>, Thierry Reding <thierry.reding@...il.com>, Sam Ravnborg <sam@...nborg.org>, David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>, dri-devel <dri-devel@...ts.freedesktop.org>, LKML <linux-kernel@...r.kernel.org>, Rob Herring <robh+dt@...nel.org>, Sandeep Panda <spanda@...eaurora.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>, Hsin-Yi Wang <hsinyi@...omium.org>, Rob Herring <robh@...nel.org> Subject: Re: [PATCH v2 1/2] drm/panel-edp: Add eDP innolux panel support Hi, On Tue, Jun 28, 2022 at 2:00 AM Rex Nie <rexnie3@...il.com> wrote: > > Add support for the 14" innolux,n140hca-eac eDP panel. > > Signed-off-by: Rex Nie <rexnie3@...il.com> > Acked-by: Hsin-Yi Wang <hsinyi@...omium.org> > --- > drivers/gpu/drm/panel/panel-edp.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > index 3626469c4cc2..2a8fcdffe80c 100644 > --- a/drivers/gpu/drm/panel/panel-edp.c > +++ b/drivers/gpu/drm/panel/panel-edp.c > @@ -1355,6 +1355,29 @@ static const struct panel_desc innolux_n125hce_gn1 = { > }, > }; > > +static const struct display_timing innolux_n140hca_eac_timing = { > + .pixelclock = { 72600000, 76420000, 80240000 }, > + .hactive = { 1920, 1920, 1920 }, > + .hfront_porch = { 80, 80, 80 }, > + .hback_porch = { 190, 190, 190 }, > + .hsync_len = { 60, 60, 60 }, > + .vactive = { 1080, 1080, 1080 }, > + .vfront_porch = { 6, 6, 6 }, > + .vback_porch = { 38, 38, 38 }, > + .vsync_len = { 8, 8, 8 }, > + .flags = DISPLAY_FLAGS_VSYNC_LOW | DISPLAY_FLAGS_HSYNC_LOW, > +}; A few questions: 1. If I'm doing my math right, you're saying that this panel runs at 30 Hz refresh rate. Truly? While I won't dismiss that as impossible, it feels unlikely. Specifically: In [2]: 72600000 / ((1920 + 80 + 190 + 60) * (1080 + 6 + 38 + 8)) Out[2]: 28.50412249705536 In [3]: 80240000 / ((1920 + 80 + 190 + 60) * (1080 + 6 + 38 + 8)) Out[3]: 31.503729878288183 NOTE: I managed to dig up a datasheet for this panel and the datasheet I have shows it as a 60 Hz refresh rate panel. 2. You're using the "struct display_timing" here instead of the "struct drm_display_mode". That can be OK, but can I ask why exactly? 3. Are you sure you need to add this entry? Moving forward I'm trying to encourage people to use the generic "edp-panel". Mostly you'd want to add a hardcoded panel here if: a) Devices have already shipped using hardcoded timings and we don't want to risk breaking something in the field with "edp-panel". b) You're trying to support some eDP controller that can't handle the generic "edp-panel". In this case I'm OK with landing changes but I'd strongly encourage you to update the controller to handle things. > +static const struct panel_desc innolux_n140hca_eac = { > + .timings = &innolux_n140hca_eac_timing, > + .num_timings = 1, > + .bpc = 6, Is it really 6 bpc? The datasheet I dug up claims 16777216 colors which would be 8 bpc. The EDID from that same datasheet also claims 8 bpc. > + .size = { > + .width = 309, > + .height = 174, > + }, Where are your delays? I know in old code these were hard to figure out from the panel spec, but the kernel doc comments now translate it into standard eDP terminology so this should be trivially easy for you to provide.
Powered by blists - more mailing lists