diff mbox series

[8/8] drm: mxsfb: Add support for the bus-width DT property

Message ID 20200813012910.13576-9-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series drm: mxsfb: Allow overriding bus width | expand

Commit Message

Laurent Pinchart Aug. 13, 2020, 1:29 a.m. UTC 
A new bus-width DT property has been introduced in the bindings to allow
overriding the bus width. Support it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/mxsfb/mxsfb_drv.h |  2 ++
 drivers/gpu/drm/mxsfb/mxsfb_kms.c |  8 ++++++--
 3 files changed, 34 insertions(+), 2 deletions(-)

Comments

Sam Ravnborg Aug. 16, 2020, 7:46 a.m. UTC | #1 
Hi Laurent.

On Thu, Aug 13, 2020 at 04:29:10AM +0300, Laurent Pinchart wrote:
> A new bus-width DT property has been introduced in the bindings to allow
> overriding the bus width. Support it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

We already reads the bus-width in following files in drm:
atmel-hlcdc/atmel_hlcdc_output.c bridge/ti-tfp410.c

So this calls for a common helper to do this like:

int drm_of_bus_fmt(const struct device_node *ep)
{
}

This helper could then read bus-width, data-shift (if relevant)
and return the relevant bus format.

I can see that bridge/ti-tfp410.c assumes 12 equals
MEDIA_BUS_FMT_RGB888_2X12_LE where as mxsfb assumes 12 is
MEDIA_BUS_FMT_RGB444_1X12.
I do not know why neither how to handle this difference.
Maybe this is something to do with DVI as this is what tfp410
seems to support.

	Sam


> ---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 26 ++++++++++++++++++++++++++
>  drivers/gpu/drm/mxsfb/mxsfb_drv.h |  2 ++
>  drivers/gpu/drm/mxsfb/mxsfb_kms.c |  8 ++++++--
>  3 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 8c549c3931af..fab3aae8cf73 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -95,10 +95,36 @@ static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb)
>  {
>  	struct drm_device *drm = mxsfb->drm;
>  	struct drm_connector_list_iter iter;
> +	struct device_node *ep;
>  	struct drm_panel *panel;
>  	struct drm_bridge *bridge;
> +	u32 bus_width = 0;
>  	int ret;
>  
> +	ep = of_graph_get_endpoint_by_regs(drm->dev->of_node, 0, 0);
> +	if (!ep)
> +		return -ENODEV;
> +
> +	of_property_read_u32(ep, "bus-width", &bus_width);
> +	of_node_put(ep);
> +
> +	switch (bus_width) {
> +	case 16:
> +		mxsfb->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
> +		break;
> +	case 18:
> +		mxsfb->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
> +		break;
> +	case 24:
> +		mxsfb->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +		break;
> +	case 0:
> +		break;
> +	default:
> +		DRM_DEV_ERROR(drm->dev, "Invalid bus-width %u", bus_width);
> +		return -ENODEV;
> +	}
> +
>  	ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, &panel,
>  					  &bridge);
>  	if (ret)
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> index 399d23e91ed1..c4f7a8a0c891 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> @@ -32,6 +32,8 @@ struct mxsfb_drm_private {
>  	struct clk			*clk_axi;
>  	struct clk			*clk_disp_axi;
>  
> +	u32				bus_format;
> +
>  	struct drm_device		*drm;
>  	struct {
>  		struct drm_plane	primary;
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> index b721b8b262ce..6d512f346918 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> @@ -50,11 +50,15 @@ static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb)
>  {
>  	struct drm_device *drm = mxsfb->drm;
>  	const u32 format = mxsfb->crtc.primary->state->fb->format->format;
> -	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +	u32 bus_format;
>  	u32 ctrl, ctrl1;
>  
> -	if (mxsfb->connector->display_info.num_bus_formats)
> +	if (mxsfb->bus_format)
> +		bus_format = mxsfb->bus_format;
> +	else if (mxsfb->connector->display_info.num_bus_formats)
>  		bus_format = mxsfb->connector->display_info.bus_formats[0];
> +	else
> +		bus_format = MEDIA_BUS_FMT_RGB888_1X24;
>  
>  	DRM_DEV_DEBUG_DRIVER(drm->dev, "Using bus_format: 0x%08X\n",
>  			     bus_format);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Laurent Pinchart Aug. 17, 2020, 12:29 a.m. UTC | #2 
Hi Sam,

On Sun, Aug 16, 2020 at 09:46:30AM +0200, Sam Ravnborg wrote:
> On Thu, Aug 13, 2020 at 04:29:10AM +0300, Laurent Pinchart wrote:
> > A new bus-width DT property has been introduced in the bindings to allow
> > overriding the bus width. Support it.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> We already reads the bus-width in following files in drm:
> atmel-hlcdc/atmel_hlcdc_output.c bridge/ti-tfp410.c
> 
> So this calls for a common helper to do this like:
> 
> int drm_of_bus_fmt(const struct device_node *ep)
> {
> }
> 
> This helper could then read bus-width, data-shift (if relevant)
> and return the relevant bus format.
> 
> I can see that bridge/ti-tfp410.c assumes 12 equals
> MEDIA_BUS_FMT_RGB888_2X12_LE where as mxsfb assumes 12 is
> MEDIA_BUS_FMT_RGB444_1X12.
> I do not know why neither how to handle this difference.
> Maybe this is something to do with DVI as this is what tfp410
> seems to support.

I think the mapping from bus width to format is device-specific, I'm not
sure a common helper is possible.

And now that I think about this, I wonder if data-shift = <2> wouldn't
be a better option than bus-width = <24>, as a 18-bpp panel will always
be connected with 18 lines, not 24. One issue, however, is that for the
565 case, I'd need a different data-shift value for R/B and for G, which
isn't possible. I don't think that turning data-shift into a
multi-integers property is worth it though. Given that DT properties are
interpreted in the context of a particular binding using bus-width could
be fine here, but I'm open to better alternatives if someone has a good
proposal.

> > ---
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 26 ++++++++++++++++++++++++++
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.h |  2 ++
> >  drivers/gpu/drm/mxsfb/mxsfb_kms.c |  8 ++++++--
> >  3 files changed, 34 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > index 8c549c3931af..fab3aae8cf73 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > @@ -95,10 +95,36 @@ static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb)
> >  {
> >  	struct drm_device *drm = mxsfb->drm;
> >  	struct drm_connector_list_iter iter;
> > +	struct device_node *ep;
> >  	struct drm_panel *panel;
> >  	struct drm_bridge *bridge;
> > +	u32 bus_width = 0;
> >  	int ret;
> >  
> > +	ep = of_graph_get_endpoint_by_regs(drm->dev->of_node, 0, 0);
> > +	if (!ep)
> > +		return -ENODEV;
> > +
> > +	of_property_read_u32(ep, "bus-width", &bus_width);
> > +	of_node_put(ep);
> > +
> > +	switch (bus_width) {
> > +	case 16:
> > +		mxsfb->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
> > +		break;
> > +	case 18:
> > +		mxsfb->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
> > +		break;
> > +	case 24:
> > +		mxsfb->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> > +		break;
> > +	case 0:
> > +		break;
> > +	default:
> > +		DRM_DEV_ERROR(drm->dev, "Invalid bus-width %u", bus_width);
> > +		return -ENODEV;
> > +	}
> > +
> >  	ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, &panel,
> >  					  &bridge);
> >  	if (ret)
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > index 399d23e91ed1..c4f7a8a0c891 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> > @@ -32,6 +32,8 @@ struct mxsfb_drm_private {
> >  	struct clk			*clk_axi;
> >  	struct clk			*clk_disp_axi;
> >  
> > +	u32				bus_format;
> > +
> >  	struct drm_device		*drm;
> >  	struct {
> >  		struct drm_plane	primary;
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> > index b721b8b262ce..6d512f346918 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> > @@ -50,11 +50,15 @@ static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb)
> >  {
> >  	struct drm_device *drm = mxsfb->drm;
> >  	const u32 format = mxsfb->crtc.primary->state->fb->format->format;
> > -	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> > +	u32 bus_format;
> >  	u32 ctrl, ctrl1;
> >  
> > -	if (mxsfb->connector->display_info.num_bus_formats)
> > +	if (mxsfb->bus_format)
> > +		bus_format = mxsfb->bus_format;
> > +	else if (mxsfb->connector->display_info.num_bus_formats)
> >  		bus_format = mxsfb->connector->display_info.bus_formats[0];
> > +	else
> > +		bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> >  
> >  	DRM_DEV_DEBUG_DRIVER(drm->dev, "Using bus_format: 0x%08X\n",
> >  			     bus_format);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 8c549c3931af..fab3aae8cf73 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -95,10 +95,36 @@  static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb)
 {
 	struct drm_device *drm = mxsfb->drm;
 	struct drm_connector_list_iter iter;
+	struct device_node *ep;
 	struct drm_panel *panel;
 	struct drm_bridge *bridge;
+	u32 bus_width = 0;
 	int ret;
 
+	ep = of_graph_get_endpoint_by_regs(drm->dev->of_node, 0, 0);
+	if (!ep)
+		return -ENODEV;
+
+	of_property_read_u32(ep, "bus-width", &bus_width);
+	of_node_put(ep);
+
+	switch (bus_width) {
+	case 16:
+		mxsfb->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
+		break;
+	case 18:
+		mxsfb->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
+		break;
+	case 24:
+		mxsfb->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
+	case 0:
+		break;
+	default:
+		DRM_DEV_ERROR(drm->dev, "Invalid bus-width %u", bus_width);
+		return -ENODEV;
+	}
+
 	ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, &panel,
 					  &bridge);
 	if (ret)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index 399d23e91ed1..c4f7a8a0c891 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -32,6 +32,8 @@  struct mxsfb_drm_private {
 	struct clk			*clk_axi;
 	struct clk			*clk_disp_axi;
 
+	u32				bus_format;
+
 	struct drm_device		*drm;
 	struct {
 		struct drm_plane	primary;
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
index b721b8b262ce..6d512f346918 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
@@ -50,11 +50,15 @@  static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb)
 {
 	struct drm_device *drm = mxsfb->drm;
 	const u32 format = mxsfb->crtc.primary->state->fb->format->format;
-	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	u32 bus_format;
 	u32 ctrl, ctrl1;
 
-	if (mxsfb->connector->display_info.num_bus_formats)
+	if (mxsfb->bus_format)
+		bus_format = mxsfb->bus_format;
+	else if (mxsfb->connector->display_info.num_bus_formats)
 		bus_format = mxsfb->connector->display_info.bus_formats[0];
+	else
+		bus_format = MEDIA_BUS_FMT_RGB888_1X24;
 
 	DRM_DEV_DEBUG_DRIVER(drm->dev, "Using bus_format: 0x%08X\n",
 			     bus_format);