diff mbox series

[05/21] drm: mxsfb: Clarify format and bus width configuration

Message ID 20200309195216.31042-6-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series drm: mxsfb: Add i.MX7 support | expand

Commit Message

Laurent Pinchart March 9, 2020, 7:52 p.m. UTC
Replace the convoluted way to set the format and bus width through
difficult to read macros with more explicit ones. Also remove the
outdated comment related to the limitations on bus width setting as it
doesn't apply anymore (the bus width can be specified through the
display_info bus format).

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 17 +++++------------
 drivers/gpu/drm/mxsfb/mxsfb_regs.h | 17 ++++++++---------
 2 files changed, 13 insertions(+), 21 deletions(-)

Comments

Stefan Agner March 23, 2020, 9:42 p.m. UTC | #1
On 2020-03-09 20:52, Laurent Pinchart wrote:
> Replace the convoluted way to set the format and bus width through
> difficult to read macros with more explicit ones. Also remove the
> outdated comment related to the limitations on bus width setting as it

Nice catch.

> doesn't apply anymore (the bus width can be specified through the
> display_info bus format).

Reviewed-by: Stefan Agner <stefan@agner.ch>

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 17 +++++------------
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h | 17 ++++++++---------
>  2 files changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> index b69ace8bf526..8b6339316929 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> @@ -52,13 +52,6 @@ static int mxsfb_set_pixel_fmt(struct
> mxsfb_drm_private *mxsfb)
>  
>  	ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
>  
> -	/*
> -	 * WARNING: The bus width, CTRL_SET_BUS_WIDTH(), is configured to
> -	 * match the selected mode here. This differs from the original
> -	 * MXSFB driver, which had the option to configure the bus width
> -	 * to arbitrary value. This limitation should not pose an issue.
> -	 */
> -
>  	/* CTRL1 contains IRQ config and status bits, preserve those. */
>  	ctrl1 = readl(mxsfb->base + LCDC_CTRL1);
>  	ctrl1 &= CTRL1_CUR_FRAME_DONE_IRQ_EN | CTRL1_CUR_FRAME_DONE_IRQ;
> @@ -66,12 +59,12 @@ static int mxsfb_set_pixel_fmt(struct
> mxsfb_drm_private *mxsfb)
>  	switch (format) {
>  	case DRM_FORMAT_RGB565:
>  		dev_dbg(drm->dev, "Setting up RGB565 mode\n");
> -		ctrl |= CTRL_SET_WORD_LENGTH(0);
> +		ctrl |= CTRL_WORD_LENGTH_16;
>  		ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
>  		break;
>  	case DRM_FORMAT_XRGB8888:
>  		dev_dbg(drm->dev, "Setting up XRGB8888 mode\n");
> -		ctrl |= CTRL_SET_WORD_LENGTH(3);
> +		ctrl |= CTRL_WORD_LENGTH_24;
>  		/* Do not use packed pixels = one pixel per word instead. */
>  		ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7);
>  		break;
> @@ -104,13 +97,13 @@ static void mxsfb_set_bus_fmt(struct
> mxsfb_drm_private *mxsfb)
>  	reg &= ~CTRL_BUS_WIDTH_MASK;
>  	switch (bus_format) {
>  	case MEDIA_BUS_FMT_RGB565_1X16:
> -		reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_16BIT);
> +		reg |= CTRL_BUS_WIDTH_16;
>  		break;
>  	case MEDIA_BUS_FMT_RGB666_1X18:
> -		reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_18BIT);
> +		reg |= CTRL_BUS_WIDTH_18;
>  		break;
>  	case MEDIA_BUS_FMT_RGB888_1X24:
> -		reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_24BIT);
> +		reg |= CTRL_BUS_WIDTH_24;
>  		break;
>  	default:
>  		dev_err(drm->dev, "Unknown media bus format %d\n", bus_format);
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> index 78e6cb754712..8ebb52bb1b46 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> @@ -34,11 +34,15 @@
>  #define CTRL_VSYNC_MODE			BIT(18)
>  #define CTRL_DOTCLK_MODE		BIT(17)
>  #define CTRL_DATA_SELECT		BIT(16)
> -#define CTRL_SET_BUS_WIDTH(x)		(((x) & 0x3) << 10)
> -#define CTRL_GET_BUS_WIDTH(x)		(((x) >> 10) & 0x3)
> +#define CTRL_BUS_WIDTH_16		(0 << 10)
> +#define CTRL_BUS_WIDTH_8		(1 << 10)
> +#define CTRL_BUS_WIDTH_18		(2 << 10)
> +#define CTRL_BUS_WIDTH_24		(3 << 10)
>  #define CTRL_BUS_WIDTH_MASK		(0x3 << 10)
> -#define CTRL_SET_WORD_LENGTH(x)		(((x) & 0x3) << 8)
> -#define CTRL_GET_WORD_LENGTH(x)		(((x) >> 8) & 0x3)
> +#define CTRL_WORD_LENGTH_16		(0 << 8)
> +#define CTRL_WORD_LENGTH_8		(1 << 8)
> +#define CTRL_WORD_LENGTH_18		(2 << 8)
> +#define CTRL_WORD_LENGTH_24		(3 << 8)
>  #define CTRL_MASTER			BIT(5)
>  #define CTRL_DF16			BIT(3)
>  #define CTRL_DF18			BIT(2)
> @@ -91,9 +95,4 @@
>  #define MXSFB_MAX_XRES			0xffff
>  #define MXSFB_MAX_YRES			0xffff
>  
> -#define STMLCDIF_8BIT  1 /* pixel data bus to the display is of 8 bit width */
> -#define STMLCDIF_16BIT 0 /* pixel data bus to the display is of 16 bit width */
> -#define STMLCDIF_18BIT 2 /* pixel data bus to the display is of 18 bit width */
> -#define STMLCDIF_24BIT 3 /* pixel data bus to the display is of 24 bit width */
> -
>  #endif /* __MXSFB_REGS_H__ */
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index b69ace8bf526..8b6339316929 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -52,13 +52,6 @@  static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb)
 
 	ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
 
-	/*
-	 * WARNING: The bus width, CTRL_SET_BUS_WIDTH(), is configured to
-	 * match the selected mode here. This differs from the original
-	 * MXSFB driver, which had the option to configure the bus width
-	 * to arbitrary value. This limitation should not pose an issue.
-	 */
-
 	/* CTRL1 contains IRQ config and status bits, preserve those. */
 	ctrl1 = readl(mxsfb->base + LCDC_CTRL1);
 	ctrl1 &= CTRL1_CUR_FRAME_DONE_IRQ_EN | CTRL1_CUR_FRAME_DONE_IRQ;
@@ -66,12 +59,12 @@  static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb)
 	switch (format) {
 	case DRM_FORMAT_RGB565:
 		dev_dbg(drm->dev, "Setting up RGB565 mode\n");
-		ctrl |= CTRL_SET_WORD_LENGTH(0);
+		ctrl |= CTRL_WORD_LENGTH_16;
 		ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
 		break;
 	case DRM_FORMAT_XRGB8888:
 		dev_dbg(drm->dev, "Setting up XRGB8888 mode\n");
-		ctrl |= CTRL_SET_WORD_LENGTH(3);
+		ctrl |= CTRL_WORD_LENGTH_24;
 		/* Do not use packed pixels = one pixel per word instead. */
 		ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7);
 		break;
@@ -104,13 +97,13 @@  static void mxsfb_set_bus_fmt(struct mxsfb_drm_private *mxsfb)
 	reg &= ~CTRL_BUS_WIDTH_MASK;
 	switch (bus_format) {
 	case MEDIA_BUS_FMT_RGB565_1X16:
-		reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_16BIT);
+		reg |= CTRL_BUS_WIDTH_16;
 		break;
 	case MEDIA_BUS_FMT_RGB666_1X18:
-		reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_18BIT);
+		reg |= CTRL_BUS_WIDTH_18;
 		break;
 	case MEDIA_BUS_FMT_RGB888_1X24:
-		reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_24BIT);
+		reg |= CTRL_BUS_WIDTH_24;
 		break;
 	default:
 		dev_err(drm->dev, "Unknown media bus format %d\n", bus_format);
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index 78e6cb754712..8ebb52bb1b46 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -34,11 +34,15 @@ 
 #define CTRL_VSYNC_MODE			BIT(18)
 #define CTRL_DOTCLK_MODE		BIT(17)
 #define CTRL_DATA_SELECT		BIT(16)
-#define CTRL_SET_BUS_WIDTH(x)		(((x) & 0x3) << 10)
-#define CTRL_GET_BUS_WIDTH(x)		(((x) >> 10) & 0x3)
+#define CTRL_BUS_WIDTH_16		(0 << 10)
+#define CTRL_BUS_WIDTH_8		(1 << 10)
+#define CTRL_BUS_WIDTH_18		(2 << 10)
+#define CTRL_BUS_WIDTH_24		(3 << 10)
 #define CTRL_BUS_WIDTH_MASK		(0x3 << 10)
-#define CTRL_SET_WORD_LENGTH(x)		(((x) & 0x3) << 8)
-#define CTRL_GET_WORD_LENGTH(x)		(((x) >> 8) & 0x3)
+#define CTRL_WORD_LENGTH_16		(0 << 8)
+#define CTRL_WORD_LENGTH_8		(1 << 8)
+#define CTRL_WORD_LENGTH_18		(2 << 8)
+#define CTRL_WORD_LENGTH_24		(3 << 8)
 #define CTRL_MASTER			BIT(5)
 #define CTRL_DF16			BIT(3)
 #define CTRL_DF18			BIT(2)
@@ -91,9 +95,4 @@ 
 #define MXSFB_MAX_XRES			0xffff
 #define MXSFB_MAX_YRES			0xffff
 
-#define STMLCDIF_8BIT  1 /* pixel data bus to the display is of 8 bit width */
-#define STMLCDIF_16BIT 0 /* pixel data bus to the display is of 16 bit width */
-#define STMLCDIF_18BIT 2 /* pixel data bus to the display is of 18 bit width */
-#define STMLCDIF_24BIT 3 /* pixel data bus to the display is of 24 bit width */
-
 #endif /* __MXSFB_REGS_H__ */