RE: [PATCH v3 4/7] drm: Define DRM_MODE_CONNECTOR_PARALLEL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

Thank you for your feedback!

> From: devicetree-owner@xxxxxxxxxxxxxxx <devicetree-owner@xxxxxxxxxxxxxxx> On Behalf Of Laurent Pinchart
> Sent: 07 November 2019 20:47
> Subject: Re: [PATCH v3 4/7] drm: Define DRM_MODE_CONNECTOR_PARALLEL
> 
> Hi Fabrizio,
> 
> (CC'ing Sam)
> 
> Thank you for the patch.
> 
> On Thu, Nov 07, 2019 at 08:11:00PM +0000, Fabrizio Castro wrote:
> > The existing DRM_MODE_CONNECTOR_ definitions don't seem to
> > describe the connector for RGB/Parallel embedded displays,
> > hence add DRM_MODE_CONNECTOR_PARALLEL.
> 
> Please, no. We already have too many connector types for panels, when
> userspace should really not care. DRM_MODE_CONNECTOR_LVDS,
> DRM_MODE_CONNECTOR_eDP, DRM_MODE_CONNECTOR_DSI, DRM_MODE_CONNECTOR_DPI
> and probably DRM_MODE_CONNECTOR_SPI should have been
> DRM_MODE_CONNECTOR_PANEL.
> 
> This has been discussed in [1]. Let's instead define a
> DRM_MODE_CONNECTOR_PANEL, possibly as an alias to one of the existing
> types, and deprecate the other types.
> 
> [1] https://www.spinics.net/lists/dri-devel/msg224638.html

Thank you for the pointer and the for the details. That clarifies things a lot.
In my case, as you mentioned in the patch to simple panel, I can use an
existing definition, therefore I think it's best if DRM_MODE_CONNECTOR_PANEL
gets added when there is a valid use case.

Thanks,
Fab

> 
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@xxxxxxxxxxxxxx>
> >
> > ---
> > v2->v3:
> > * New patch
> > ---
> >  drivers/gpu/drm/drm_connector.c | 1 +
> >  include/uapi/drm/drm_mode.h     | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 2166000..b233029 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -93,6 +93,7 @@ static struct drm_conn_prop_enum_list drm_connector_enum_list[] = {
> >  	{ DRM_MODE_CONNECTOR_DPI, "DPI" },
> >  	{ DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
> >  	{ DRM_MODE_CONNECTOR_SPI, "SPI" },
> > +	{ DRM_MODE_CONNECTOR_PARALLEL, "Parallel" },
> >  };
> >
> >  void drm_connector_ida_init(void)
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 735c8cf..5852f47 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -362,6 +362,7 @@ enum drm_mode_subconnector {
> >  #define DRM_MODE_CONNECTOR_DPI		17
> >  #define DRM_MODE_CONNECTOR_WRITEBACK	18
> >  #define DRM_MODE_CONNECTOR_SPI		19
> > +#define DRM_MODE_CONNECTOR_PARALLEL	20
> >
> >  struct drm_mode_get_connector {
> >
> 
> --
> Regards,
> 
> Laurent Pinchart




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux