| Message ID | 20200617222703.17080-8-digetx@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support DRM bridges on NVIDIA Tegra | expand |
Hi Dmitry On Thu, Jun 18, 2020 at 01:27:03AM +0300, Dmitry Osipenko wrote: > The DRM panel bridge core requires connector type to be set up properly, > otherwise it rejects the panel. The missing connector type problem popped > up while I was trying to wrap CLAA070WP03XG panel into a DRM bridge in > order to test whether panel's rotation property work properly using > panel-simple driver on NVIDIA Tegra30 Nexus 7 tablet device, which uses > CLAA070WP03XG display panel. > > The NVIDIA Tegra DRM driver recently gained DRM bridges support for the > RGB output and now driver wraps directly-connected panels into DRM bridge. > Hence all panels should have connector type set properly now, otherwise > the panel's wrapping fails. > > This patch adds missing connector types for the LVDS panels that are found > on NVIDIA Tegra devices: > > 1. AUO B101AW03 > 2. Chunghwa CLAA070WP03XG > 3. Chunghwa CLAA101WA01A > 4. Chunghwa CLAA101WB01 > 5. EDT ET057090DHU > 6. Innolux N156BGE L21 > 7. Samsung LTN101NT05 > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> Very good to have this fixed. I went ahead and pushed this commit to drm-misc-next as it is really independent from the rest of the series. Sam > --- > drivers/gpu/drm/panel/panel-simple.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index 6764ac630e22..9eb2dbb7bfa6 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -687,6 +687,7 @@ static const struct panel_desc auo_b101aw03 = { > .width = 223, > .height = 125, > }, > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > static const struct display_timing auo_b101ean01_timing = { > @@ -1340,6 +1341,7 @@ static const struct panel_desc chunghwa_claa070wp03xg = { > .width = 94, > .height = 150, > }, > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > static const struct drm_display_mode chunghwa_claa101wa01a_mode = { > @@ -1362,6 +1364,7 @@ static const struct panel_desc chunghwa_claa101wa01a = { > .width = 220, > .height = 120, > }, > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > static const struct drm_display_mode chunghwa_claa101wb01_mode = { > @@ -1384,6 +1387,7 @@ static const struct panel_desc chunghwa_claa101wb01 = { > .width = 223, > .height = 125, > }, > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > static const struct drm_display_mode dataimage_scf0700c48ggu18_mode = { > @@ -1573,6 +1577,7 @@ static const struct panel_desc edt_et057090dhu = { > }, > .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE, > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > static const struct drm_display_mode edt_etm0700g0dh6_mode = { > @@ -2055,6 +2060,7 @@ static const struct panel_desc innolux_n156bge_l21 = { > .width = 344, > .height = 193, > }, > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > static const struct drm_display_mode innolux_p120zdg_bf1_mode = { > @@ -3001,6 +3007,7 @@ static const struct panel_desc samsung_ltn101nt05 = { > .width = 223, > .height = 125, > }, > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > static const struct drm_display_mode samsung_ltn140at29_301_mode = { > -- > 2.26.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Sam and Dmitry, On Sat, Jun 20, 2020 at 01:21:32PM +0200, Sam Ravnborg wrote: > On Thu, Jun 18, 2020 at 01:27:03AM +0300, Dmitry Osipenko wrote: > > The DRM panel bridge core requires connector type to be set up properly, > > otherwise it rejects the panel. The missing connector type problem popped > > up while I was trying to wrap CLAA070WP03XG panel into a DRM bridge in > > order to test whether panel's rotation property work properly using > > panel-simple driver on NVIDIA Tegra30 Nexus 7 tablet device, which uses > > CLAA070WP03XG display panel. > > > > The NVIDIA Tegra DRM driver recently gained DRM bridges support for the > > RGB output and now driver wraps directly-connected panels into DRM bridge. > > Hence all panels should have connector type set properly now, otherwise > > the panel's wrapping fails. > > > > This patch adds missing connector types for the LVDS panels that are found > > on NVIDIA Tegra devices: > > > > 1. AUO B101AW03 > > 2. Chunghwa CLAA070WP03XG > > 3. Chunghwa CLAA101WA01A > > 4. Chunghwa CLAA101WB01 > > 5. EDT ET057090DHU > > 6. Innolux N156BGE L21 > > 7. Samsung LTN101NT05 > > > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > > Very good to have this fixed. > I went ahead and pushed this commit to drm-misc-next as it is really > independent from the rest of the series. > > > --- > > drivers/gpu/drm/panel/panel-simple.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > > index 6764ac630e22..9eb2dbb7bfa6 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -687,6 +687,7 @@ static const struct panel_desc auo_b101aw03 = { > > .width = 223, > > .height = 125, > > }, > > + .connector_type = DRM_MODE_CONNECTOR_LVDS, Note that, for LVDS panels, the bus_format field is mandatory. This panel, for instance, according to http://www.vslcd.com/Specification/B101AW03%20V.0.pdf, uses MEDIA_BUS_FMT_RGB666_1X7X3_SPWG (see https://linuxtv.org/downloads/v4l-dvb-apis/userspace-api/v4l/subdev-formats.html#v4l2-mbus-pixelcode). The panels below need to be investigated similarly. > > }; > > > > static const struct display_timing auo_b101ean01_timing = { > > @@ -1340,6 +1341,7 @@ static const struct panel_desc chunghwa_claa070wp03xg = { > > .width = 94, > > .height = 150, > > }, > > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct drm_display_mode chunghwa_claa101wa01a_mode = { > > @@ -1362,6 +1364,7 @@ static const struct panel_desc chunghwa_claa101wa01a = { > > .width = 220, > > .height = 120, > > }, > > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct drm_display_mode chunghwa_claa101wb01_mode = { > > @@ -1384,6 +1387,7 @@ static const struct panel_desc chunghwa_claa101wb01 = { > > .width = 223, > > .height = 125, > > }, > > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct drm_display_mode dataimage_scf0700c48ggu18_mode = { > > @@ -1573,6 +1577,7 @@ static const struct panel_desc edt_et057090dhu = { > > }, > > .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > > .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE, > > + .connector_type = DRM_MODE_CONNECTOR_LVDS, This contradicts .bus_format and .bus_flags that hint that the panel is a DPI panel, not an LVDS panel. According to https://www.lcdtek.co.uk/dwpdf/ET057090DHU-RoHS.pdf, this isn't an LVDS panel. I'm worried enough research hasn't gone into this patch, and I'd prefer reverting it until we check each panel individually. > > }; > > > > static const struct drm_display_mode edt_etm0700g0dh6_mode = { > > @@ -2055,6 +2060,7 @@ static const struct panel_desc innolux_n156bge_l21 = { > > .width = 344, > > .height = 193, > > }, > > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct drm_display_mode innolux_p120zdg_bf1_mode = { > > @@ -3001,6 +3007,7 @@ static const struct panel_desc samsung_ltn101nt05 = { > > .width = 223, > > .height = 125, > > }, > > + .connector_type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > static const struct drm_display_mode samsung_ltn140at29_301_mode = {
20.06.2020 14:49, Laurent Pinchart пишет: > Hi Sam and Dmitry, > > On Sat, Jun 20, 2020 at 01:21:32PM +0200, Sam Ravnborg wrote: >> On Thu, Jun 18, 2020 at 01:27:03AM +0300, Dmitry Osipenko wrote: >>> The DRM panel bridge core requires connector type to be set up properly, >>> otherwise it rejects the panel. The missing connector type problem popped >>> up while I was trying to wrap CLAA070WP03XG panel into a DRM bridge in >>> order to test whether panel's rotation property work properly using >>> panel-simple driver on NVIDIA Tegra30 Nexus 7 tablet device, which uses >>> CLAA070WP03XG display panel. >>> >>> The NVIDIA Tegra DRM driver recently gained DRM bridges support for the >>> RGB output and now driver wraps directly-connected panels into DRM bridge. >>> Hence all panels should have connector type set properly now, otherwise >>> the panel's wrapping fails. >>> >>> This patch adds missing connector types for the LVDS panels that are found >>> on NVIDIA Tegra devices: >>> >>> 1. AUO B101AW03 >>> 2. Chunghwa CLAA070WP03XG >>> 3. Chunghwa CLAA101WA01A >>> 4. Chunghwa CLAA101WB01 >>> 5. EDT ET057090DHU >>> 6. Innolux N156BGE L21 >>> 7. Samsung LTN101NT05 >>> >>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> >> Very good to have this fixed. >> I went ahead and pushed this commit to drm-misc-next as it is really >> independent from the rest of the series. >> >>> --- >>> drivers/gpu/drm/panel/panel-simple.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c >>> index 6764ac630e22..9eb2dbb7bfa6 100644 >>> --- a/drivers/gpu/drm/panel/panel-simple.c >>> +++ b/drivers/gpu/drm/panel/panel-simple.c >>> @@ -687,6 +687,7 @@ static const struct panel_desc auo_b101aw03 = { >>> .width = 223, >>> .height = 125, >>> }, >>> + .connector_type = DRM_MODE_CONNECTOR_LVDS, > > Note that, for LVDS panels, the bus_format field is mandatory. This > panel, for instance, according to > http://www.vslcd.com/Specification/B101AW03%20V.0.pdf, uses > MEDIA_BUS_FMT_RGB666_1X7X3_SPWG (see > https://linuxtv.org/downloads/v4l-dvb-apis/userspace-api/v4l/subdev-formats.html#v4l2-mbus-pixelcode). > The panels below need to be investigated similarly. Okay! I'll add the missing field in v9. >>> }; >>> >>> static const struct display_timing auo_b101ean01_timing = { >>> @@ -1340,6 +1341,7 @@ static const struct panel_desc chunghwa_claa070wp03xg = { >>> .width = 94, >>> .height = 150, >>> }, >>> + .connector_type = DRM_MODE_CONNECTOR_LVDS, >>> }; >>> >>> static const struct drm_display_mode chunghwa_claa101wa01a_mode = { >>> @@ -1362,6 +1364,7 @@ static const struct panel_desc chunghwa_claa101wa01a = { >>> .width = 220, >>> .height = 120, >>> }, >>> + .connector_type = DRM_MODE_CONNECTOR_LVDS, >>> }; >>> >>> static const struct drm_display_mode chunghwa_claa101wb01_mode = { >>> @@ -1384,6 +1387,7 @@ static const struct panel_desc chunghwa_claa101wb01 = { >>> .width = 223, >>> .height = 125, >>> }, >>> + .connector_type = DRM_MODE_CONNECTOR_LVDS, >>> }; >>> >>> static const struct drm_display_mode dataimage_scf0700c48ggu18_mode = { >>> @@ -1573,6 +1577,7 @@ static const struct panel_desc edt_et057090dhu = { >>> }, >>> .bus_format = MEDIA_BUS_FMT_RGB666_1X18, >>> .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE, >>> + .connector_type = DRM_MODE_CONNECTOR_LVDS, > > This contradicts .bus_format and .bus_flags that hint that the panel is > a DPI panel, not an LVDS panel. According to > https://www.lcdtek.co.uk/dwpdf/ET057090DHU-RoHS.pdf, this isn't an LVDS > panel. > > I'm worried enough research hasn't gone into this patch, and I'd prefer > reverting it until we check each panel individually. Hello Sam and Laurent, Oops! Good catch! Indeed, I blindly set the LVDS type to all these panels. Please revert this patch, I'll double check each panel and prepare an updated version of this patch. Thank you very much for the review!
Hi Dmitry > > Oops! Good catch! Yep, thanks Laurent. Should have taken a better look before applying. > Indeed, I blindly set the LVDS type to all these > panels. Please revert this patch, I'll double check each panel and > prepare an updated version of this patch. Thank you very much for the > review! If you can prepare a fix within a few days then lets wait for that. I will do a better review next time. Sam
20.06.2020 17:31, Sam Ravnborg пишет: > Hi Dmitry > >> >> Oops! Good catch! > Yep, thanks Laurent. Should have taken a better look before applying. > >> Indeed, I blindly set the LVDS type to all these >> panels. Please revert this patch, I'll double check each panel and >> prepare an updated version of this patch. Thank you very much for the >> review! > > If you can prepare a fix within a few days then lets wait for that. > I will do a better review next time. Hello Sam, I should be able to make it later today or tomorrow. Could you please clarify what do you mean by the fix, do you what it to be as an additional patch on top of the applied one or a new version of the patch?
Hi Dmitry On Sat, Jun 20, 2020 at 06:05:37PM +0300, Dmitry Osipenko wrote: > 20.06.2020 17:31, Sam Ravnborg пишет: > > Hi Dmitry > > > >> > >> Oops! Good catch! > > Yep, thanks Laurent. Should have taken a better look before applying. > > > >> Indeed, I blindly set the LVDS type to all these > >> panels. Please revert this patch, I'll double check each panel and > >> prepare an updated version of this patch. Thank you very much for the > >> review! > > > > If you can prepare a fix within a few days then lets wait for that. > > I will do a better review next time. > > Hello Sam, > > I should be able to make it later today or tomorrow. Could you please > clarify what do you mean by the fix, do you what it to be as an > additional patch on top of the applied one or a new version of the patch? An additional patch on top of the one applied. It shall carry a proper fixes: tag like this: Fixes: 94f07917ebe8 ("drm/panel-simple: Add missing connector type for some panels") Cc: Dmitry Osipenko <digetx@gmail.com> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: dri-devel@lists.freedesktop.org Sam
20.06.2020 18:30, Sam Ravnborg пишет: > Hi Dmitry > On Sat, Jun 20, 2020 at 06:05:37PM +0300, Dmitry Osipenko wrote: >> 20.06.2020 17:31, Sam Ravnborg пишет: >>> Hi Dmitry >>> >>>> >>>> Oops! Good catch! >>> Yep, thanks Laurent. Should have taken a better look before applying. >>> >>>> Indeed, I blindly set the LVDS type to all these >>>> panels. Please revert this patch, I'll double check each panel and >>>> prepare an updated version of this patch. Thank you very much for the >>>> review! >>> >>> If you can prepare a fix within a few days then lets wait for that. >>> I will do a better review next time. >> >> Hello Sam, >> >> I should be able to make it later today or tomorrow. Could you please >> clarify what do you mean by the fix, do you what it to be as an >> additional patch on top of the applied one or a new version of the patch? > An additional patch on top of the one applied. > It shall carry a proper fixes: tag like this: > > Fixes: 94f07917ebe8 ("drm/panel-simple: Add missing connector type for some panels") > Cc: Dmitry Osipenko <digetx@gmail.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: dri-devel@lists.freedesktop.org Okay!
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 6764ac630e22..9eb2dbb7bfa6 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -687,6 +687,7 @@ static const struct panel_desc auo_b101aw03 = { .width = 223, .height = 125, }, + .connector_type = DRM_MODE_CONNECTOR_LVDS, }; static const struct display_timing auo_b101ean01_timing = { @@ -1340,6 +1341,7 @@ static const struct panel_desc chunghwa_claa070wp03xg = { .width = 94, .height = 150, }, + .connector_type = DRM_MODE_CONNECTOR_LVDS, }; static const struct drm_display_mode chunghwa_claa101wa01a_mode = { @@ -1362,6 +1364,7 @@ static const struct panel_desc chunghwa_claa101wa01a = { .width = 220, .height = 120, }, + .connector_type = DRM_MODE_CONNECTOR_LVDS, }; static const struct drm_display_mode chunghwa_claa101wb01_mode = { @@ -1384,6 +1387,7 @@ static const struct panel_desc chunghwa_claa101wb01 = { .width = 223, .height = 125, }, + .connector_type = DRM_MODE_CONNECTOR_LVDS, }; static const struct drm_display_mode dataimage_scf0700c48ggu18_mode = { @@ -1573,6 +1577,7 @@ static const struct panel_desc edt_et057090dhu = { }, .bus_format = MEDIA_BUS_FMT_RGB666_1X18, .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE, + .connector_type = DRM_MODE_CONNECTOR_LVDS, }; static const struct drm_display_mode edt_etm0700g0dh6_mode = { @@ -2055,6 +2060,7 @@ static const struct panel_desc innolux_n156bge_l21 = { .width = 344, .height = 193, }, + .connector_type = DRM_MODE_CONNECTOR_LVDS, }; static const struct drm_display_mode innolux_p120zdg_bf1_mode = { @@ -3001,6 +3007,7 @@ static const struct panel_desc samsung_ltn101nt05 = { .width = 223, .height = 125, }, + .connector_type = DRM_MODE_CONNECTOR_LVDS, }; static const struct drm_display_mode samsung_ltn140at29_301_mode = {
The DRM panel bridge core requires connector type to be set up properly, otherwise it rejects the panel. The missing connector type problem popped up while I was trying to wrap CLAA070WP03XG panel into a DRM bridge in order to test whether panel's rotation property work properly using panel-simple driver on NVIDIA Tegra30 Nexus 7 tablet device, which uses CLAA070WP03XG display panel. The NVIDIA Tegra DRM driver recently gained DRM bridges support for the RGB output and now driver wraps directly-connected panels into DRM bridge. Hence all panels should have connector type set properly now, otherwise the panel's wrapping fails. This patch adds missing connector types for the LVDS panels that are found on NVIDIA Tegra devices: 1. AUO B101AW03 2. Chunghwa CLAA070WP03XG 3. Chunghwa CLAA101WA01A 4. Chunghwa CLAA101WB01 5. EDT ET057090DHU 6. Innolux N156BGE L21 7. Samsung LTN101NT05 Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpu/drm/panel/panel-simple.c | 7 +++++++ 1 file changed, 7 insertions(+)