diff mbox series

[RFC,12/16] drm/armada: add bus-width property to the output endpoint

Message ID 20181218153742.1313125-13-lkundrak@v3.sk (mailing list archive)
State New, archived
Headers show
Series Armada DRM support for OLPC XO-1.75 laptop | expand

Commit Message

Lubomir Rintel Dec. 18, 2018, 3:37 p.m. UTC
This makes it possible to choose a different pixel format for the
endpoint. Modelled after what other LCD controllers use, including
marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps more.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/gpu/drm/armada/armada_crtc.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Comments

Russell King (Oracle) Jan. 3, 2019, 1:15 p.m. UTC | #1
On Tue, Dec 18, 2018 at 04:37:38PM +0100, Lubomir Rintel wrote:
> This makes it possible to choose a different pixel format for the
> endpoint. Modelled after what other LCD controllers use, including
> marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps more.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  drivers/gpu/drm/armada/armada_crtc.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> index da9360688b55..5400fb925bcd 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.c
> +++ b/drivers/gpu/drm/armada/armada_crtc.c
> @@ -724,6 +724,8 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
>  	struct armada_private *priv = drm->dev_private;
>  	struct armada_crtc *dcrtc;
>  	struct drm_plane *primary;
> +	struct device_node *endpoint;
> +	u32 bus_width = 24;
>  	void __iomem *base;
>  	int ret;
>  
> @@ -744,8 +746,30 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
>  	dcrtc->base = base;
>  	dcrtc->num = drm->mode_config.num_crtc;
>  	dcrtc->clk = ERR_PTR(-EINVAL);
> -	dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
>  	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
> +
> +	endpoint = of_get_next_child(port, NULL);
> +	of_property_read_u32(endpoint, "bus-width", &bus_width);
> +	of_node_put(endpoint);
> +
> +	switch (bus_width) {
> +	case 12:
> +		dcrtc->cfg_dumb_ctrl = DUMB12_RGB444_0;
> +		break;
> +	case 16:
> +		dcrtc->cfg_dumb_ctrl = DUMB16_RGB565_0;
> +		break;
> +	case 18:
> +		dcrtc->cfg_dumb_ctrl = DUMB18_RGB666_0;
> +		break;
> +	case 24:
> +		dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> +		break;
> +	default:
> +		DRM_ERROR("unsupported bus width: %d\n", bus_width);
> +		return -EINVAL;
> +	}
> +

This needs to default to 24bpp if the property is not present.
Lubomir Rintel Jan. 3, 2019, 1:20 p.m. UTC | #2
On Thu, 2019-01-03 at 13:15 +0000, Russell King - ARM Linux wrote:
> On Tue, Dec 18, 2018 at 04:37:38PM +0100, Lubomir Rintel wrote:
> > This makes it possible to choose a different pixel format for the
> > endpoint. Modelled after what other LCD controllers use, including
> > marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps
> > more.
> > 
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> >  drivers/gpu/drm/armada/armada_crtc.c | 26
> > +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/armada/armada_crtc.c
> > b/drivers/gpu/drm/armada/armada_crtc.c
> > index da9360688b55..5400fb925bcd 100644
> > --- a/drivers/gpu/drm/armada/armada_crtc.c
> > +++ b/drivers/gpu/drm/armada/armada_crtc.c
> > @@ -724,6 +724,8 @@ static int armada_drm_crtc_create(struct
> > drm_device *drm, struct device *dev,
> >  	struct armada_private *priv = drm->dev_private;
> >  	struct armada_crtc *dcrtc;
> >  	struct drm_plane *primary;
> > +	struct device_node *endpoint;
> > +	u32 bus_width = 24;
> >  	void __iomem *base;
> >  	int ret;
> >  
> > @@ -744,8 +746,30 @@ static int armada_drm_crtc_create(struct
> > drm_device *drm, struct device *dev,
> >  	dcrtc->base = base;
> >  	dcrtc->num = drm->mode_config.num_crtc;
> >  	dcrtc->clk = ERR_PTR(-EINVAL);
> > -	dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> >  	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
> > +
> > +	endpoint = of_get_next_child(port, NULL);
> > +	of_property_read_u32(endpoint, "bus-width", &bus_width);
> > +	of_node_put(endpoint);
> > +
> > +	switch (bus_width) {
> > +	case 12:
> > +		dcrtc->cfg_dumb_ctrl = DUMB12_RGB444_0;
> > +		break;
> > +	case 16:
> > +		dcrtc->cfg_dumb_ctrl = DUMB16_RGB565_0;
> > +		break;
> > +	case 18:
> > +		dcrtc->cfg_dumb_ctrl = DUMB18_RGB666_0;
> > +		break;
> > +	case 24:
> > +		dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> > +		break;
> > +	default:
> > +		DRM_ERROR("unsupported bus width: %d\n", bus_width);
> > +		return -EINVAL;
> > +	}
> > +
> 
> This needs to default to 24bpp if the property is not present.

It actually does: bus_width is initialized to 24 and
of_property_read_u32() does not change it if the property is not
present.

Perhaps the style is poor and I should just set the bus_width=24 in the
case's default branch instead.

Lubo
Russell King (Oracle) Jan. 3, 2019, 1:21 p.m. UTC | #3
On Thu, Jan 03, 2019 at 02:20:00PM +0100, Lubomir Rintel wrote:
> On Thu, 2019-01-03 at 13:15 +0000, Russell King - ARM Linux wrote:
> > On Tue, Dec 18, 2018 at 04:37:38PM +0100, Lubomir Rintel wrote:
> > > This makes it possible to choose a different pixel format for the
> > > endpoint. Modelled after what other LCD controllers use, including
> > > marvell,pxa2xx-lcdc and atmel,hlcdc-display-controller and perhaps
> > > more.
> > > 
> > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > ---
> > >  drivers/gpu/drm/armada/armada_crtc.c | 26
> > > +++++++++++++++++++++++++-
> > >  1 file changed, 25 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/armada/armada_crtc.c
> > > b/drivers/gpu/drm/armada/armada_crtc.c
> > > index da9360688b55..5400fb925bcd 100644
> > > --- a/drivers/gpu/drm/armada/armada_crtc.c
> > > +++ b/drivers/gpu/drm/armada/armada_crtc.c
> > > @@ -724,6 +724,8 @@ static int armada_drm_crtc_create(struct
> > > drm_device *drm, struct device *dev,
> > >  	struct armada_private *priv = drm->dev_private;
> > >  	struct armada_crtc *dcrtc;
> > >  	struct drm_plane *primary;
> > > +	struct device_node *endpoint;
> > > +	u32 bus_width = 24;
> > >  	void __iomem *base;
> > >  	int ret;
> > >  
> > > @@ -744,8 +746,30 @@ static int armada_drm_crtc_create(struct
> > > drm_device *drm, struct device *dev,
> > >  	dcrtc->base = base;
> > >  	dcrtc->num = drm->mode_config.num_crtc;
> > >  	dcrtc->clk = ERR_PTR(-EINVAL);
> > > -	dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> > >  	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
> > > +
> > > +	endpoint = of_get_next_child(port, NULL);
> > > +	of_property_read_u32(endpoint, "bus-width", &bus_width);
> > > +	of_node_put(endpoint);
> > > +
> > > +	switch (bus_width) {
> > > +	case 12:
> > > +		dcrtc->cfg_dumb_ctrl = DUMB12_RGB444_0;
> > > +		break;
> > > +	case 16:
> > > +		dcrtc->cfg_dumb_ctrl = DUMB16_RGB565_0;
> > > +		break;
> > > +	case 18:
> > > +		dcrtc->cfg_dumb_ctrl = DUMB18_RGB666_0;
> > > +		break;
> > > +	case 24:
> > > +		dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
> > > +		break;
> > > +	default:
> > > +		DRM_ERROR("unsupported bus width: %d\n", bus_width);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > 
> > This needs to default to 24bpp if the property is not present.
> 
> It actually does: bus_width is initialized to 24 and
> of_property_read_u32() does not change it if the property is not
> present.
> 
> Perhaps the style is poor and I should just set the bus_width=24 in the
> case's default branch instead.

No, that's fine then.  Thanks for pointing that out.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index da9360688b55..5400fb925bcd 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -724,6 +724,8 @@  static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
 	struct armada_private *priv = drm->dev_private;
 	struct armada_crtc *dcrtc;
 	struct drm_plane *primary;
+	struct device_node *endpoint;
+	u32 bus_width = 24;
 	void __iomem *base;
 	int ret;
 
@@ -744,8 +746,30 @@  static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
 	dcrtc->base = base;
 	dcrtc->num = drm->mode_config.num_crtc;
 	dcrtc->clk = ERR_PTR(-EINVAL);
-	dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
 	dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
+
+	endpoint = of_get_next_child(port, NULL);
+	of_property_read_u32(endpoint, "bus-width", &bus_width);
+	of_node_put(endpoint);
+
+	switch (bus_width) {
+	case 12:
+		dcrtc->cfg_dumb_ctrl = DUMB12_RGB444_0;
+		break;
+	case 16:
+		dcrtc->cfg_dumb_ctrl = DUMB16_RGB565_0;
+		break;
+	case 18:
+		dcrtc->cfg_dumb_ctrl = DUMB18_RGB666_0;
+		break;
+	case 24:
+		dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0;
+		break;
+	default:
+		DRM_ERROR("unsupported bus width: %d\n", bus_width);
+		return -EINVAL;
+	}
+
 	spin_lock_init(&dcrtc->irq_lock);
 	dcrtc->irq_ena = CLEAN_SPU_IRQ_ISR;