Re: [PATCH 1/2] drm/panel: Add and fill drm_panel type field
Hi Laurent, On 23/08/2019 at 03:40, Laurent Pinchart wrote: > Add a type field to the drm_panel structure to report the panel type, > using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS, > eDP, DSI and DPI). This will be used to initialise the corresponding > connector type. With Microchip/Atmel driver, we mainly (only) use the "Unknown" type of connector because our hardware simply uses RGB wires in parallel. Should we move to another connector type (maybe now that it's created and it was not, back when we chose the "Unknown" one)? What would be the consequences if we move (silently?) to another type and particularly on the command line argument like the ones we currently use: "Unknown-1:800x480-16"? Thanks for your insight on this. Best regards, Nicolas > Update all panel drivers to fill the new field. > > Signed-off-by: Laurent Pinchart > --- [..] -- Nicolas Ferre ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/panel: Add and fill drm_panel type field
Hi Sam,
On Fri, Aug 23, 2019 at 06:49:49AM +0200, Sam Ravnborg wrote:
> Hi Laurent.
>
> Thanks for looking into this, but you will not be happy in a minute...
Could be worse :-)
> On Fri, Aug 23, 2019 at 04:40:32AM +0300, Laurent Pinchart wrote:
> > Add a type field to the drm_panel structure to report the panel type,
> > using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS,
> > eDP, DSI and DPI).
>
> > This will be used to initialise the corresponding connector type.
> Ohh, that explains what this should be used for.
> I had missed that we have the panel when we create the drm_connector.
>
> (I had seen we had to use the connector type to match a panel with a
> connector or something like that).
>
> > diff --git a/drivers/gpu/drm/panel/panel-arm-versatile.c
> > b/drivers/gpu/drm/panel/panel-arm-versatile.c
> > index 5f72c922a04b..5c335fc1632b 100644
> > --- a/drivers/gpu/drm/panel/panel-arm-versatile.c
> > +++ b/drivers/gpu/drm/panel/panel-arm-versatile.c
> > @@ -353,6 +353,7 @@ static int versatile_panel_probe(struct platform_device
> > *pdev)
> > drm_panel_init(>panel);
> > vpanel->panel.dev = dev;
> > vpanel->panel.funcs = _panel_drm_funcs;
> > + vpanel->panel.type = DRM_MODE_CONNECTOR_DPI;
>
> The pattern where we call a simple init like here,
> and then have a set of mandatory assignments right after
> is not a good way to help the driver writer.
>
> We should instead do:
>
> drm_panel_init(>panel, dev, _panel_drm_funcs,
> DRM_MODE_CONNECTOR_DPI);
>
> Then all drm_panel users are forced to supply the init parameters,
> and we do not secretly rely on some defaults for panels that do not have it.
> Also new panels will fail to build if they do not specify the new field.
I'll fix that.
> This may also allow us to chatch that:
>
> > diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> > b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> > index b5b14aa059ea..cac074939e2c 100644
> > --- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> > +++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> > @@ -428,6 +428,7 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c,
> >
> > ts->base.dev = dev;
> > ts->base.funcs = _touchscreen_funcs;
> > + ts->base.type = DRM_MODE_CONNECTOR_DSI;
>
> forgets to call drm_panel_init()
>
>
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c
> > b/drivers/gpu/drm/panel/panel-simple.c
> > index 28fa6ba7b767..6d5d0c51e97e 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -94,6 +94,7 @@ struct panel_desc {
> >
> > u32 bus_format;
> > u32 bus_flags;
> > + unsigned int type;
> > };
> >
> > struct panel_simple {
> > @@ -467,6 +468,7 @@ static int panel_simple_probe(struct device *dev, const
> > struct panel_desc *desc)
> > drm_panel_init(>base);
> > panel->base.dev = dev;
> > panel->base.funcs = _simple_funcs;
> > + panel->base.type = desc->type ? desc->type : DRM_MODE_CONNECTOR_DPI;
> >
> > err = drm_panel_add(>base);
> > if (err < 0)
> > @@ -833,6 +835,7 @@ static const struct panel_desc auo_g133han01 = {
> > .unprepare = 1000,
> > },
> > .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA,
> > + .type = DRM_MODE_CONNECTOR_LVDS,
> > };
> >
> > static const struct display_timing auo_g185han01_timings = {
> > @@ -862,6 +865,7 @@ static const struct panel_desc auo_g185han01 = {
> > .unprepare = 1000,
> > },
> > .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> > + .type = DRM_MODE_CONNECTOR_LVDS,
> > };
> >
> > static const struct display_timing auo_p320hvn03_timings = {
> > @@ -890,6 +894,7 @@ static const struct panel_desc auo_p320hvn03 = {
> > .unprepare = 500,
> > },
> > .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> > + .type = DRM_MODE_CONNECTOR_LVDS,
> > };
> >
> > static const struct drm_display_mode auo_t215hvn01_mode = {
> > @@ -1205,6 +1210,7 @@ static const struct panel_desc dlc_dlc0700yzg_1 = {
> > .disable = 200,
> > },
> > .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> > + .type = DRM_MODE_CONNECTOR_LVDS,
> > };
> >
> > static const struct display_timing dlc_dlc1010gig_timing = {
> > @@ -1235,6 +1241,7 @@ static const struct panel_desc dlc_dlc1010gig = {
> > .unprepare = 60,
> > },
> > .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> > + .type = DRM_MODE_CONNECTOR_LVDS,
> > };
> >
> > static const struct drm_display_mode edt_et035012dm6_mode = {
> > @@ -1501,6 +1508,7 @@ static const struct panel_desc hannstar_hsd070pww1 = {
> > .height = 94,
> > },
> > .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
> > + .type = DRM_MODE_CONNECTOR_LVDS,
> > };
> >
> > static const struct display_timing hannstar_hsd100pxn1_timing = {
> > @@ -1525,6 +1533,7 @@ static const struct panel_desc hannstar_hsd100pxn1 = {
> >
Re: [PATCH 1/2] drm/panel: Add and fill drm_panel type field
On Fri, 23 Aug 2019 07:30:07 + wrote: > Hi Laurent, > > On 23/08/2019 at 03:40, Laurent Pinchart wrote: > > Add a type field to the drm_panel structure to report the panel type, > > using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS, > > eDP, DSI and DPI). This will be used to initialise the corresponding > > connector type. > > With Microchip/Atmel driver, we mainly (only) use the "Unknown" type of > connector because our hardware simply uses RGB wires in parallel. > > Should we move to another connector type (maybe now that it's created > and it was not, back when we chose the "Unknown" one)? I confirm, DRM_MODE_ENCODER_DPI was not defined when I switched from _LVDS (which was wrong) to _Unknown. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/panel: Add and fill drm_panel type field
Add a type field to the drm_panel structure to report the panel type,
using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS,
eDP, DSI and DPI). This will be used to initialise the corresponding
connector type.
Update all panel drivers to fill the new field.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/panel/panel-arm-versatile.c | 1 +
.../drm/panel/panel-feiyang-fy07024di26a30d.c | 1 +
drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 1 +
drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 1 +
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 1 +
.../gpu/drm/panel/panel-jdi-lt070me05000.c| 1 +
.../drm/panel/panel-kingdisplay-kd097d04.c| 1 +
drivers/gpu/drm/panel/panel-lg-lb035q02.c | 1 +
drivers/gpu/drm/panel/panel-lg-lg4573.c | 1 +
drivers/gpu/drm/panel/panel-lvds.c| 1 +
drivers/gpu/drm/panel/panel-nec-nl8048hl11.c | 1 +
drivers/gpu/drm/panel/panel-novatek-nt39016.c | 1 +
.../drm/panel/panel-olimex-lcd-olinuxino.c| 1 +
.../gpu/drm/panel/panel-orisetech-otm8009a.c | 1 +
.../drm/panel/panel-osd-osd101t2587-53ts.c| 1 +
.../drm/panel/panel-panasonic-vvx10f034n00.c | 1 +
.../drm/panel/panel-raspberrypi-touchscreen.c | 1 +
drivers/gpu/drm/panel/panel-raydium-rm67191.c | 1 +
drivers/gpu/drm/panel/panel-raydium-rm68200.c | 1 +
.../drm/panel/panel-rocktech-jh057n00900.c| 1 +
drivers/gpu/drm/panel/panel-ronbo-rb070d30.c | 1 +
drivers/gpu/drm/panel/panel-samsung-ld9040.c | 1 +
drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 1 +
drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 1 +
.../gpu/drm/panel/panel-samsung-s6e63j0x03.c | 1 +
drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 1 +
drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c | 1 +
drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 1 +
.../gpu/drm/panel/panel-sharp-lq101r1sx01.c | 1 +
.../gpu/drm/panel/panel-sharp-ls037v7dw01.c | 1 +
.../gpu/drm/panel/panel-sharp-ls043t1le01.c | 1 +
drivers/gpu/drm/panel/panel-simple.c | 23 +++
drivers/gpu/drm/panel/panel-sitronix-st7701.c | 1 +
.../gpu/drm/panel/panel-sitronix-st7789v.c| 1 +
drivers/gpu/drm/panel/panel-sony-acx565akm.c | 1 +
drivers/gpu/drm/panel/panel-tpo-td028ttec1.c | 1 +
drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 1 +
drivers/gpu/drm/panel/panel-tpo-tpg110.c | 1 +
drivers/gpu/drm/panel/panel-truly-nt35597.c | 1 +
include/drm/drm_panel.h | 7 ++
40 files changed, 68 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-arm-versatile.c
b/drivers/gpu/drm/panel/panel-arm-versatile.c
index 5f72c922a04b..5c335fc1632b 100644
--- a/drivers/gpu/drm/panel/panel-arm-versatile.c
+++ b/drivers/gpu/drm/panel/panel-arm-versatile.c
@@ -353,6 +353,7 @@ static int versatile_panel_probe(struct platform_device
*pdev)
drm_panel_init(>panel);
vpanel->panel.dev = dev;
vpanel->panel.funcs = _panel_drm_funcs;
+ vpanel->panel.type = DRM_MODE_CONNECTOR_DPI;
return drm_panel_add(>panel);
}
diff --git a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
index dabf59e0f56f..58894a4bf02c 100644
--- a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
+++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
@@ -207,6 +207,7 @@ static int feiyang_dsi_probe(struct mipi_dsi_device *dsi)
drm_panel_init(>panel);
ctx->panel.dev = >dev;
ctx->panel.funcs = _funcs;
+ ctx->panel.type = DRM_MODE_CONNECTOR_DSI;
ctx->dvdd = devm_regulator_get(>dev, "dvdd");
if (IS_ERR(ctx->dvdd)) {
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
index 3c58f63adbf7..10a6f4ab53e2 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c
@@ -898,6 +898,7 @@ static int ili9322_probe(struct spi_device *spi)
drm_panel_init(>panel);
ili->panel.dev = dev;
ili->panel.funcs = _drm_funcs;
+ ili->panel.type = DRM_MODE_CONNECTOR_DPI;
return drm_panel_add(>panel);
}
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
index 3ad4a46c4e94..360b87ff62a1 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
@@ -436,6 +436,7 @@ static int ili9881c_dsi_probe(struct mipi_dsi_device *dsi)
drm_panel_init(>panel);
ctx->panel.dev = >dev;
ctx->panel.funcs = _funcs;
+ ctx->panel.type = DRM_MODE_CONNECTOR_DSI;
ctx->power = devm_regulator_get(>dev, "power");
if (IS_ERR(ctx->power)) {
diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index d92d1c98878c..00b24662ebb5 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++
