diff mbox series

[v3,4/7] drm: Define DRM_MODE_CONNECTOR_PARALLEL

Message ID 1573157463-14070-5-git-send-email-fabrizio.castro@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Kieran Bingham
Headers show
Series Add LCD panel support to iwg20d | expand

Commit Message

Fabrizio Castro Nov. 7, 2019, 8:11 p.m. UTC
The existing DRM_MODE_CONNECTOR_ definitions don't seem to
describe the connector for RGB/Parallel embedded displays,
hence add DRM_MODE_CONNECTOR_PARALLEL.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v2->v3:
* New patch
---
 drivers/gpu/drm/drm_connector.c | 1 +
 include/uapi/drm/drm_mode.h     | 1 +
 2 files changed, 2 insertions(+)

Comments

Laurent Pinchart Nov. 7, 2019, 8:46 p.m. UTC | #1
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

> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> ---
> 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 {
>
Fabrizio Castro Nov. 8, 2019, 9:32 a.m. UTC | #2
Hi Laurent,

Thank you for your feedback!

> From: devicetree-owner@vger.kernel.org <devicetree-owner@vger.kernel.org> 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@bp.renesas.com>
> >
> > ---
> > 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
diff mbox series

Patch

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 {