Re: [PATCH 04/10] drm/mxsfb: Update mxsfb with additional pixel formats

2019-01-14 Thread Guido Günther
Hi,
On Wed, Jan 09, 2019 at 02:13:43PM +, Robert Chiras wrote:
> Since version 4 of eLCDIF, there are some registers that can do
> transformations on the input data, like re-arranging the pixel
> components. By doing that, we can support more pixel formats.
> This patch adds support for X/ABGR and RGBX/A. Although, the local alpha
> is not supported by eLCDIF, the alpha pixel formats were added to the
> supported pixel formats but it will be ignored. This was necessary since
> there are systems (like Android) that requires such pixel formats.
> 
> Signed-off-by: Robert Chiras 
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 108 
> -
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c  |  27 +++---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.h  |   4 +-
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h | 100 --
>  4 files changed, 169 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c 
> b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> index f0648ce..6aa8804 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> @@ -48,15 +48,35 @@ static u32 set_hsync_pulse_width(struct mxsfb_drm_private 
> *mxsfb, u32 val)
>   mxsfb->devdata->hs_wdth_shift;
>  }
>  
> +/*  Print Four-character-code (FOURCC) */
> +static char *fourcc_to_str(u32 fmt)
> +{
> + /* Use 10 chars so we can simultaneously print two codes */
> + static char code[10], *c = [0];
> +
> + if (c == [10])
> + c = [0];
> +
> + *(c++) = (unsigned char)(fmt & 0xff);
> + *(c++) = (unsigned char)((fmt >> 8) & 0xff);
> + *(c++) = (unsigned char)((fmt >> 16) & 0xff);
> + *(c++) = (unsigned char)((fmt >> 24) & 0xff);
> + *(c++) = '\0';
> +
> + return (c - 5);
> +}
> +
>  /* Setup the MXSFB registers for decoding the pixels out of the framebuffer 
> */
> -static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb)
> +static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb, bool update)
>  {
>   struct drm_crtc *crtc = >pipe.crtc;
>   struct drm_device *drm = crtc->dev;
>   const u32 format = crtc->primary->state->fb->format->format;
> - u32 ctrl, ctrl1;
> + u32 ctrl = 0, ctrl1 = 0;
> + bool bgr_format = true;
>  
> - ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
> + if (!update)
> + ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
>  
>   /*
>* WARNING: The bus width, CTRL_SET_BUS_WIDTH(), is configured to
> @@ -65,31 +85,69 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private 
> *mxsfb)
>* 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;
> + if (!update) {
> + /* 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;
> + }
> +
> + DRM_DEV_DEBUG_DRIVER(drm->dev, "Setting up %s mode\n",
> + fourcc_to_str(format));
> +
> + /* Do some clean-up that we might have from a previous mode */
> + ctrl &= ~CTRL_SHIFT_DIR(1);
> + ctrl &= ~CTRL_SHIFT_NUM(0x3f);
> + if (mxsfb->devdata->ipversion >= 4)
> + writel(CTRL2_ODD_LINE_PATTERN(0x7) |
> +CTRL2_EVEN_LINE_PATTERN(0x7),
> +mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
>  
>   switch (format) {
>   case DRM_FORMAT_RGB565:
> - dev_dbg(drm->dev, "Setting up RGB565 mode\n");
>   ctrl |= CTRL_SET_WORD_LENGTH(0);
>   ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
>   break;
> + case DRM_FORMAT_RGBX:
> + case DRM_FORMAT_RGBA:
> + /* RGBX - > 0RGB */
> + ctrl |= CTRL_SHIFT_DIR(1);
> + ctrl |= CTRL_SHIFT_NUM(8);
> + bgr_format = false;
> + /* Fall through */
> + case DRM_FORMAT_XBGR:
> + case DRM_FORMAT_ABGR:
> + if (bgr_format) {
> + if (mxsfb->devdata->ipversion < 4)
> + goto err;
> + writel(CTRL2_ODD_LINE_PATTERN(0x5) |
> +CTRL2_EVEN_LINE_PATTERN(0x5),
> +mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
> + }
> + /* Fall through */
>   case DRM_FORMAT_XRGB:
> - dev_dbg(drm->dev, "Setting up XRGB mode\n");
> + case DRM_FORMAT_ARGB:
>   ctrl |= CTRL_SET_WORD_LENGTH(3);
>   /* Do not use packed pixels = one pixel per word instead. */
>   ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7);
>   break;
>   default:
> - dev_err(drm->dev, "Unhandled pixel format %08x\n", format);
> -   

[PATCH 04/10] drm/mxsfb: Update mxsfb with additional pixel formats

2019-01-10 Thread Robert Chiras
Since version 4 of eLCDIF, there are some registers that can do
transformations on the input data, like re-arranging the pixel
components. By doing that, we can support more pixel formats.
This patch adds support for X/ABGR and RGBX/A. Although, the local alpha
is not supported by eLCDIF, the alpha pixel formats were added to the
supported pixel formats but it will be ignored. This was necessary since
there are systems (like Android) that requires such pixel formats.

Signed-off-by: Robert Chiras 
---
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 108 -
 drivers/gpu/drm/mxsfb/mxsfb_drv.c  |  27 +++---
 drivers/gpu/drm/mxsfb/mxsfb_drv.h  |   4 +-
 drivers/gpu/drm/mxsfb/mxsfb_regs.h | 100 --
 4 files changed, 169 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c 
b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
index f0648ce..6aa8804 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -48,15 +48,35 @@ static u32 set_hsync_pulse_width(struct mxsfb_drm_private 
*mxsfb, u32 val)
mxsfb->devdata->hs_wdth_shift;
 }
 
+/*  Print Four-character-code (FOURCC) */
+static char *fourcc_to_str(u32 fmt)
+{
+   /* Use 10 chars so we can simultaneously print two codes */
+   static char code[10], *c = [0];
+
+   if (c == [10])
+   c = [0];
+
+   *(c++) = (unsigned char)(fmt & 0xff);
+   *(c++) = (unsigned char)((fmt >> 8) & 0xff);
+   *(c++) = (unsigned char)((fmt >> 16) & 0xff);
+   *(c++) = (unsigned char)((fmt >> 24) & 0xff);
+   *(c++) = '\0';
+
+   return (c - 5);
+}
+
 /* Setup the MXSFB registers for decoding the pixels out of the framebuffer */
-static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb)
+static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb, bool update)
 {
struct drm_crtc *crtc = >pipe.crtc;
struct drm_device *drm = crtc->dev;
const u32 format = crtc->primary->state->fb->format->format;
-   u32 ctrl, ctrl1;
+   u32 ctrl = 0, ctrl1 = 0;
+   bool bgr_format = true;
 
-   ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
+   if (!update)
+   ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
 
/*
 * WARNING: The bus width, CTRL_SET_BUS_WIDTH(), is configured to
@@ -65,31 +85,69 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private 
*mxsfb)
 * 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;
+   if (!update) {
+   /* 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;
+   }
+
+   DRM_DEV_DEBUG_DRIVER(drm->dev, "Setting up %s mode\n",
+   fourcc_to_str(format));
+
+   /* Do some clean-up that we might have from a previous mode */
+   ctrl &= ~CTRL_SHIFT_DIR(1);
+   ctrl &= ~CTRL_SHIFT_NUM(0x3f);
+   if (mxsfb->devdata->ipversion >= 4)
+   writel(CTRL2_ODD_LINE_PATTERN(0x7) |
+  CTRL2_EVEN_LINE_PATTERN(0x7),
+  mxsfb->base + LCDC_V4_CTRL2 + REG_CLR);
 
switch (format) {
case DRM_FORMAT_RGB565:
-   dev_dbg(drm->dev, "Setting up RGB565 mode\n");
ctrl |= CTRL_SET_WORD_LENGTH(0);
ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
break;
+   case DRM_FORMAT_RGBX:
+   case DRM_FORMAT_RGBA:
+   /* RGBX - > 0RGB */
+   ctrl |= CTRL_SHIFT_DIR(1);
+   ctrl |= CTRL_SHIFT_NUM(8);
+   bgr_format = false;
+   /* Fall through */
+   case DRM_FORMAT_XBGR:
+   case DRM_FORMAT_ABGR:
+   if (bgr_format) {
+   if (mxsfb->devdata->ipversion < 4)
+   goto err;
+   writel(CTRL2_ODD_LINE_PATTERN(0x5) |
+  CTRL2_EVEN_LINE_PATTERN(0x5),
+  mxsfb->base + LCDC_V4_CTRL2 + REG_SET);
+   }
+   /* Fall through */
case DRM_FORMAT_XRGB:
-   dev_dbg(drm->dev, "Setting up XRGB mode\n");
+   case DRM_FORMAT_ARGB:
ctrl |= CTRL_SET_WORD_LENGTH(3);
/* Do not use packed pixels = one pixel per word instead. */
ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7);
break;
default:
-   dev_err(drm->dev, "Unhandled pixel format %08x\n", format);
-   return -EINVAL;
+   goto err;
}
 
-   writel(ctrl1, mxsfb->base + LCDC_CTRL1);
-   writel(ctrl, mxsfb->base + LCDC_CTRL);
+