Re: DRM_MODE_CONNECTOR_PANEL? [Was: drm/panel: Add and fill drm_panel type field]
On Fri, Sep 27, 2019 at 05:28:03PM +0300, Tomi Valkeinen wrote: > On 27/09/2019 15:44, Daniel Stone wrote: > > Hi Linus, > > > > On Fri, 27 Sep 2019 at 13:37, Linus Walleij > > wrote: > > > Also the ILI9322 can actually set up gamma correction which is > > > very nice for professional applications. I haven't seen any way for > > > DRM to do gamma correction properly or any framework for it > > > to adjust and propagate gamma to/from userspace (seems like > > > another enormous task), but I am pretty sure it will be there one > > > of these days so I put in some comments and placeholders. > > > > Gamma correction has been supported since approximately the dawn of > > time with a 3x8-bit LUT. > > But, afaik, only in the display controller side. I don't think we have means > to have any properties for the panels or bridges. I guess would need some semi-elaborate negotiation dance to make sure you only correct in either the bridge/panel or in the crtc, but should be doable. I don't think we need a new property or change anything with the uapi here, just kernel internals. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM_MODE_CONNECTOR_PANEL? [Was: drm/panel: Add and fill drm_panel type field]
On 27/09/2019 15:44, Daniel Stone wrote: Hi Linus, On Fri, 27 Sep 2019 at 13:37, Linus Walleij wrote: Also the ILI9322 can actually set up gamma correction which is very nice for professional applications. I haven't seen any way for DRM to do gamma correction properly or any framework for it to adjust and propagate gamma to/from userspace (seems like another enormous task), but I am pretty sure it will be there one of these days so I put in some comments and placeholders. Gamma correction has been supported since approximately the dawn of time with a 3x8-bit LUT. But, afaik, only in the display controller side. I don't think we have means to have any properties for the panels or bridges. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM_MODE_CONNECTOR_PANEL? [Was: drm/panel: Add and fill drm_panel type field]
Hi Linus, On Fri, 27 Sep 2019 at 13:37, Linus Walleij wrote: > Also the ILI9322 can actually set up gamma correction which is > very nice for professional applications. I haven't seen any way for > DRM to do gamma correction properly or any framework for it > to adjust and propagate gamma to/from userspace (seems like > another enormous task), but I am pretty sure it will be there one > of these days so I put in some comments and placeholders. Gamma correction has been supported since approximately the dawn of time with a 3x8-bit LUT. Obviously more modern hardware has far more complex colour management. This is also supported, and is approximately described here: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties Both planes and CRTCs can have a LUT -> matrix transform -> LUT pipeline. There are also additional properties for fixed transforms, e.g. limited 16-235 <-> full 0-255 range translation. Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM_MODE_CONNECTOR_PANEL? [Was: drm/panel: Add and fill drm_panel type field]
Just a drive-by comment: On Sat, Aug 24, 2019 at 11:54 AM Sam Ravnborg wrote: > We will also start to see new drivers where there will be support > for more than one type of connector interface. > > Like for example ili9322: > - 8-bit serial RGB interface > - 24-bit parallel RGB interface > - 8-bit ITU-R BT.601 interface > - 8-bit ITU-R BT.656 interface I just found all these weird references when making the ILI9322 driver and trying to really understand what was going on in the hardware. I have this special nack (shared with many kernel developers) that I need to know what is going on. I think what is actually the case is that a *lot* of panels that we support (especially the so-called "simple" panels) are actually complex, like the ILI9322, just that many of them are undocumented, set up by ROM or bootloaders, their SPI/I2C lines forgotten about and the sum result added as a "simple panel" in panel-simple.c. This is fine I guess to get some basic support up and going, but all of a sudden someone will use that bridge/panel with some other input from the list above and ooops wrongly modeled, the DT compatible string makes no sense etc. Also the ILI9322 can actually set up gamma correction which is very nice for professional applications. I haven't seen any way for DRM to do gamma correction properly or any framework for it to adjust and propagate gamma to/from userspace (seems like another enormous task), but I am pretty sure it will be there one of these days so I put in some comments and placeholders. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM_MODE_CONNECTOR_PANEL? [Was: drm/panel: Add and fill drm_panel type field]
Hi Sam, On Sat, Aug 24, 2019 at 05:02:34PM +0300, Laurent Pinchart wrote: > On Sat, Aug 24, 2019 at 11:54:21AM +0200, Sam Ravnborg wrote: > > On Fri, Aug 23, 2019 at 10:32:44PM +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. > > > > As discussed on irc before applying this we should evaluate the > > idea to introduce DRM_MODE_CONNECTOR_PANEL > > Thank you for bringing it up. > > > Today we have: > > DRM_MODE_CONNECTOR_DPI > > DRM_MODE_CONNECTOR_DBI > > DRM_MODE_CONNECTOR_SPI > > DRM_MODE_CONNECTOR_LVDS > > > > The question if what benefit ig gives to distingush between the > > different ways panels are connected. > > > > For example for VGA and HDMI there is a physical connector involved so > > here it make good sense. > > But why should userspace care if the panel is connected via DPI or LVDS? > > > > The more generic DRM_MODE_CONNECTOR_PANEL would be equally descriptive. > > > > We will also start to see new drivers where there will be support > > for more than one type of connector interface. > > > > Like for example ili9322: > > - 8-bit serial RGB interface > > - 24-bit parallel RGB interface > > - 8-bit ITU-R BT.601 interface > > - 8-bit ITU-R BT.656 interface > > > > Adding DRM_MODE_CONNECTOR_PANEL may create an xkcd 927 situation. > > But we should then deprecate _DPI, _DBI, _SPI, _LVDS. > > _DBI doesn't exist, but we also have _eDP. > > The discussions originated from the fact that a number of drivers > incorrectly report their connector type for panels today (including > reporting DRM_MODE_CONNECTOR_Unknown), and they can't be fixed without a > risk or breaking userspace, especially as the connector type is also > used to name the connector (it would at least break command line mode > setting as reported by Nicolas Ferre). It also makes it difficult for a > driver to exhibit the right behaviour on new systems (that don't need to > care about backward compatibility) without breaking the already > supported systems. > > There were also recent discussions about adding a _DBI connector type, > which will increase this mess. I pointed out in a few IRC discussions > that I think the different connector types for panels were a mistake, > and that we should have gone for DRM_MODE_CONNECTOR_PANEL in the first > place. It's of course too late to fix this, but going forward, instead > of introducing a _DBI connector type, I think we'd be much better off > with _PANEL. Do you think it makes sense to merge this patch, regardless of the outcome of the DRM_MODE_CONNECTOR_PANEL panel discussion (which looks like a lack of discussion at the moment...) ? The patch allows exposing the correct panel type to userspace, without hindering in any way a future addition of DRM_MODE_CONNECTOR_PANEL. > >> Update all panel drivers accordingly. The panel-simple driver only > >> specifies the type for the known to be LVDS panels, while all other > >> panels are left as unknown and will be converted on a case-by-case > >> basis as they all need to be carefully reviewed. > >> > >> Signed-off-by: Laurent Pinchart > >> --- > >> Changes since v1: > >> > >> - Rename the type field to connector_type > >> - Pass the type to the drm_panel_init() function > >> - Keep connector type as unknown for non-LVDS panels in panel-simple > >> - Flag the toshiba_lt089ac29000 panel as LVDS as reported by Boris > >> --- > >> drivers/gpu/drm/drm_panel.c | 5 +++- > >> drivers/gpu/drm/panel/panel-arm-versatile.c | 3 ++- > >> .../drm/panel/panel-feiyang-fy07024di26a30d.c | 3 ++- > >> drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 3 ++- > >> drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 3 ++- > >> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 3 ++- > >> .../gpu/drm/panel/panel-jdi-lt070me05000.c| 3 ++- > >> .../drm/panel/panel-kingdisplay-kd097d04.c| 2 +- > >> drivers/gpu/drm/panel/panel-lg-lb035q02.c | 3 ++- > >> drivers/gpu/drm/panel/panel-lg-lg4573.c | 3 ++- > >> drivers/gpu/drm/panel/panel-lvds.c| 3 ++- > >> drivers/gpu/drm/panel/panel-nec-nl8048hl11.c | 3 ++- > >> drivers/gpu/drm/panel/panel-novatek-nt39016.c | 3 ++- > >> .../drm/panel/panel-olimex-lcd-olinuxino.c| 3 ++- > >> .../gpu/drm/panel/panel-orisetech-otm8009a.c | 3 ++- > >> .../drm/panel/panel-osd-osd101t2587-53ts.c| 2 +- > >> .../drm/panel/panel-panasonic-vvx10f034n00.c | 2 +- > >> .../drm/panel/panel-raspberrypi-touchscreen.c | 3 ++- > >> drivers/gpu/drm/panel/panel-raydium-rm67191.c | 3 ++- > >> drivers/gpu/drm/panel/panel-raydium-rm68200.c | 3 ++- > >> .../drm/panel/panel-rocktech-jh057n00900.c| 3 ++- > >> drivers/gpu/drm/panel/panel-ronbo-rb070d30.c | 3 ++- > >> drivers/gpu/drm/panel/panel-samsung-ld9040.c |
Re: DRM_MODE_CONNECTOR_PANEL? [Was: drm/panel: Add and fill drm_panel type field]
Hi Sam, On Sat, Aug 24, 2019 at 11:54:21AM +0200, Sam Ravnborg wrote: > On Fri, Aug 23, 2019 at 10:32:44PM +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. > > As discussed on irc before applying this we should evaluate the > idea to introduce DRM_MODE_CONNECTOR_PANEL Thank you for bringing it up. > Today we have: > DRM_MODE_CONNECTOR_DPI > DRM_MODE_CONNECTOR_DBI > DRM_MODE_CONNECTOR_SPI > DRM_MODE_CONNECTOR_LVDS > > The question if what benefit ig gives to distingush between the > different ways panels are connected. > > For example for VGA and HDMI there is a physical connector involved so > here it make good sense. > But why should userspace care if the panel is connected via DPI or LVDS? > > The more generic DRM_MODE_CONNECTOR_PANEL would be equally descriptive. > > We will also start to see new drivers where there will be support > for more than one type of connector interface. > > Like for example ili9322: > - 8-bit serial RGB interface > - 24-bit parallel RGB interface > - 8-bit ITU-R BT.601 interface > - 8-bit ITU-R BT.656 interface > > Adding DRM_MODE_CONNECTOR_PANEL may create an xkcd 927 situation. > But we should then deprecate _DPI, _DBI, _SPI, _LVDS. _DBI doesn't exist, but we also have _eDP. The discussions originated from the fact that a number of drivers incorrectly report their connector type for panels today (including reporting DRM_MODE_CONNECTOR_Unknown), and they can't be fixed without a risk or breaking userspace, especially as the connector type is also used to name the connector (it would at least break command line mode setting as reported by Nicolas Ferre). It also makes it difficult for a driver to exhibit the right behaviour on new systems (that don't need to care about backward compatibility) without breaking the already supported systems. There were also recent discussions about adding a _DBI connector type, which will increase this mess. I pointed out in a few IRC discussions that I think the different connector types for panels were a mistake, and that we should have gone for DRM_MODE_CONNECTOR_PANEL in the first place. It's of course too late to fix this, but going forward, instead of introducing a _DBI connector type, I think we'd be much better off with _PANEL. > > Update all panel drivers accordingly. The panel-simple driver only > > specifies the type for the known to be LVDS panels, while all other > > panels are left as unknown and will be converted on a case-by-case > > basis as they all need to be carefully reviewed. > > > > Signed-off-by: Laurent Pinchart > > --- > > Changes since v1: > > > > - Rename the type field to connector_type > > - Pass the type to the drm_panel_init() function > > - Keep connector type as unknown for non-LVDS panels in panel-simple > > - Flag the toshiba_lt089ac29000 panel as LVDS as reported by Boris > > --- > > drivers/gpu/drm/drm_panel.c | 5 +++- > > drivers/gpu/drm/panel/panel-arm-versatile.c | 3 ++- > > .../drm/panel/panel-feiyang-fy07024di26a30d.c | 3 ++- > > drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 3 ++- > > drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 3 ++- > > drivers/gpu/drm/panel/panel-innolux-p079zca.c | 3 ++- > > .../gpu/drm/panel/panel-jdi-lt070me05000.c| 3 ++- > > .../drm/panel/panel-kingdisplay-kd097d04.c| 2 +- > > drivers/gpu/drm/panel/panel-lg-lb035q02.c | 3 ++- > > drivers/gpu/drm/panel/panel-lg-lg4573.c | 3 ++- > > drivers/gpu/drm/panel/panel-lvds.c| 3 ++- > > drivers/gpu/drm/panel/panel-nec-nl8048hl11.c | 3 ++- > > drivers/gpu/drm/panel/panel-novatek-nt39016.c | 3 ++- > > .../drm/panel/panel-olimex-lcd-olinuxino.c| 3 ++- > > .../gpu/drm/panel/panel-orisetech-otm8009a.c | 3 ++- > > .../drm/panel/panel-osd-osd101t2587-53ts.c| 2 +- > > .../drm/panel/panel-panasonic-vvx10f034n00.c | 2 +- > > .../drm/panel/panel-raspberrypi-touchscreen.c | 3 ++- > > drivers/gpu/drm/panel/panel-raydium-rm67191.c | 3 ++- > > drivers/gpu/drm/panel/panel-raydium-rm68200.c | 3 ++- > > .../drm/panel/panel-rocktech-jh057n00900.c| 3 ++- > > drivers/gpu/drm/panel/panel-ronbo-rb070d30.c | 3 ++- > > drivers/gpu/drm/panel/panel-samsung-ld9040.c | 3 ++- > > drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 3 ++- > > drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 3 ++- > > .../gpu/drm/panel/panel-samsung-s6e63j0x03.c | 3 ++- > > drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 3 ++- > > drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c | 3 ++- > > drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 3 ++- > > .../gpu/drm/panel/panel-sharp-lq101r1sx01.c | 3 ++- > > .../gpu/drm/panel/panel-sharp-ls037v7dw01.c | 3 ++- > > .../gpu/drm/panel/panel-sharp-ls043t1le01.c
DRM_MODE_CONNECTOR_PANEL? [Was: drm/panel: Add and fill drm_panel type field]
Hi Laurent et all. On Fri, Aug 23, 2019 at 10:32:44PM +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. As discussed on irc before applying this we should evaluate the idea to introduce DRM_MODE_CONNECTOR_PANEL Today we have: DRM_MODE_CONNECTOR_DPI DRM_MODE_CONNECTOR_DBI DRM_MODE_CONNECTOR_SPI DRM_MODE_CONNECTOR_LVDS The question if what benefit ig gives to distingush between the different ways panels are connected. For example for VGA and HDMI there is a physical connector involved so here it make good sense. But why should userspace care if the panel is connected via DPI or LVDS? The more generic DRM_MODE_CONNECTOR_PANEL would be equally descriptive. We will also start to see new drivers where there will be support for more than one type of connector interface. Like for example ili9322: - 8-bit serial RGB interface - 24-bit parallel RGB interface - 8-bit ITU-R BT.601 interface - 8-bit ITU-R BT.656 interface Adding DRM_MODE_CONNECTOR_PANEL may create an xkcd 927 situation. But we should then deprecate _DPI, _DBI, _SPI, _LVDS. Sam > > Update all panel drivers accordingly. The panel-simple driver only > specifies the type for the known to be LVDS panels, while all other > panels are left as unknown and will be converted on a case-by-case > basis as they all need to be carefully reviewed. > > Signed-off-by: Laurent Pinchart > --- > Changes since v1: > > - Rename the type field to connector_type > - Pass the type to the drm_panel_init() function > - Keep connector type as unknown for non-LVDS panels in panel-simple > - Flag the toshiba_lt089ac29000 panel as LVDS as reported by Boris > --- > drivers/gpu/drm/drm_panel.c | 5 +++- > drivers/gpu/drm/panel/panel-arm-versatile.c | 3 ++- > .../drm/panel/panel-feiyang-fy07024di26a30d.c | 3 ++- > drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 3 ++- > drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 3 ++- > drivers/gpu/drm/panel/panel-innolux-p079zca.c | 3 ++- > .../gpu/drm/panel/panel-jdi-lt070me05000.c| 3 ++- > .../drm/panel/panel-kingdisplay-kd097d04.c| 2 +- > drivers/gpu/drm/panel/panel-lg-lb035q02.c | 3 ++- > drivers/gpu/drm/panel/panel-lg-lg4573.c | 3 ++- > drivers/gpu/drm/panel/panel-lvds.c| 3 ++- > drivers/gpu/drm/panel/panel-nec-nl8048hl11.c | 3 ++- > drivers/gpu/drm/panel/panel-novatek-nt39016.c | 3 ++- > .../drm/panel/panel-olimex-lcd-olinuxino.c| 3 ++- > .../gpu/drm/panel/panel-orisetech-otm8009a.c | 3 ++- > .../drm/panel/panel-osd-osd101t2587-53ts.c| 2 +- > .../drm/panel/panel-panasonic-vvx10f034n00.c | 2 +- > .../drm/panel/panel-raspberrypi-touchscreen.c | 3 ++- > drivers/gpu/drm/panel/panel-raydium-rm67191.c | 3 ++- > drivers/gpu/drm/panel/panel-raydium-rm68200.c | 3 ++- > .../drm/panel/panel-rocktech-jh057n00900.c| 3 ++- > drivers/gpu/drm/panel/panel-ronbo-rb070d30.c | 3 ++- > drivers/gpu/drm/panel/panel-samsung-ld9040.c | 3 ++- > drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 3 ++- > drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 3 ++- > .../gpu/drm/panel/panel-samsung-s6e63j0x03.c | 3 ++- > drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 3 ++- > drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c | 3 ++- > drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 3 ++- > .../gpu/drm/panel/panel-sharp-lq101r1sx01.c | 3 ++- > .../gpu/drm/panel/panel-sharp-ls037v7dw01.c | 3 ++- > .../gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +- > drivers/gpu/drm/panel/panel-simple.c | 26 ++- > drivers/gpu/drm/panel/panel-sitronix-st7701.c | 3 ++- > .../gpu/drm/panel/panel-sitronix-st7789v.c| 3 ++- > drivers/gpu/drm/panel/panel-sony-acx565akm.c | 3 ++- > drivers/gpu/drm/panel/panel-tpo-td028ttec1.c | 3 ++- > drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 3 ++- > drivers/gpu/drm/panel/panel-tpo-tpg110.c | 3 ++- > drivers/gpu/drm/panel/panel-truly-nt35597.c | 3 ++- > include/drm/drm_panel.h | 12 - > 41 files changed, 112 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index ba2fad4c9648..ed7985c0535a 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c > @@ -46,16 +46,19 @@ static LIST_HEAD(panel_list); > * @panel: DRM panel > * @dev: parent device of the panel > * @funcs: panel operations > + * @connector_type: the connector type (DRM_MODE_CONNECTOR_*) corresponding > to > + * the panel interface > * > * Initialize the panel structure for subsequent registration with > * drm_panel_add(). > */ > void drm_panel_init(struct drm_panel *panel, struct device *dev, > - const struct drm_panel_funcs *
