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
