<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size: 12pt; color: rgb(0, 0, 0); font-family: Calibri, Helvetica, sans-serif, EmojiFont, "Apple Color Emoji", "Segoe UI Emoji", NotoColorEmoji, "Segoe UI Symbol", "Android Emoji", EmojiSymbols;" dir="ltr">
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<br>
<br>
<div style="color: rgb(0, 0, 0);">
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Thierry Reding <thierry.reding@gmail.com><br>
<b>Sent:</b> Thursday, April 26, 2018 5:56 PM<br>
<b>To:</b> Robert Chiras<br>
<b>Cc:</b> dl-linux-imx; dri-devel@lists.freedesktop.org<br>
<b>Subject:</b> Re: [PATCH 2/2] drm/panel: rm67191: Add support for new bus formats</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">On Tue, Apr 03, 2018 at 01:30:01PM +0300, Robert Chiras wrote:<br>
> From: Mirela Rabulea <mirela.rabulea@nxp.com><br>
> <br>
> Do not hardcode pixel_format to 0x77 but calculate it from dsi->format.<br>
> Report all the supported bus formats in get_modes:<br>
>         MEDIA_BUS_FMT_RGB888_1X24<br>
>         MEDIA_BUS_FMT_RGB666_1X18<br>
>         MEDIA_BUS_FMT_RGB565_1X16<br>
> Change pixelclock from 120 to 132 MHz, or 16 bpp formats will not work.<br>
> <br>
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com><br>
> ---<br>
>  drivers/gpu/drm/panel/panel-raydium-rm67191.c | 33 +++++++++++++++++++++++----<br>
>  1 file changed, 28 insertions(+), 5 deletions(-)<br>
> <br>
> diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gpu/drm/panel/panel-raydium-rm67191.c<br>
> index 07b0bd4..6331fef 100644<br>
> --- a/drivers/gpu/drm/panel/panel-raydium-rm67191.c<br>
> +++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c<br>
> @@ -178,6 +178,12 @@ static const cmd_set_table manufacturer_cmd_set[] = {<br>
>        {0x51, 0x04},<br>
>  };<br>
>  <br>
> +static const u32 rad_bus_formats[] = {<br>
> +     MEDIA_BUS_FMT_RGB888_1X24,<br>
> +     MEDIA_BUS_FMT_RGB666_1X18,<br>
> +     MEDIA_BUS_FMT_RGB565_1X16,<br>
> +};<br>
> +<br>
>  struct rad_panel {<br>
>        struct drm_panel base;<br>
>        struct mipi_dsi_device *dsi;<br>
> @@ -215,11 +221,27 @@ static int rad_panel_push_cmd_list(struct mipi_dsi_device *dsi)<br>
>        return ret;<br>
>  };<br>
>  <br>
> +static int color_format_from_dsi_format(enum mipi_dsi_pixel_format format)<br>
> +{<br>
> +     switch (format) {<br>
> +     case MIPI_DSI_FMT_RGB565:<br>
> +             return 0x55;<br>
> +     case MIPI_DSI_FMT_RGB666:<br>
> +     case MIPI_DSI_FMT_RGB666_PACKED:<br>
> +             return 0x66;<br>
> +     case MIPI_DSI_FMT_RGB888:<br>
> +             return 0x77;<br>
> +     default:<br>
> +             return 0x77; /* for backward compatibility */<br>
> +     }<br>
> +};<br>
> +<br>
>  static int rad_panel_prepare(struct drm_panel *panel)<br>
>  {<br>
>        struct rad_panel *rad = to_rad_panel(panel);<br>
>        struct mipi_dsi_device *dsi = rad->dsi;<br>
>        struct device *dev = &dsi->dev;<br>
> +     int color_format = color_format_from_dsi_format(dsi->format);<br>
>        int ret;<br>
>  <br>
>        if (rad->prepared)<br>
> @@ -276,8 +298,10 @@ static int rad_panel_prepare(struct drm_panel *panel)<br>
>                DRM_DEV_ERROR(dev, "Failed to set tear scanline (%d)\n", ret);<br>
>                goto fail;<br>
>        }<br>
> -     /* Set pixel format to RGB888 */<br>
> -     ret = mipi_dsi_dcs_set_pixel_format(dsi, 0x77);<br>
> +     /* Set pixel format */<br>
> +     ret = mipi_dsi_dcs_set_pixel_format(dsi, color_format);<br>
> +     DRM_DEV_DEBUG_DRIVER(dev, "Interface color format set to 0x%x\n",<br>
> +                             color_format);<br>
>        if (ret < 0) {<br>
>                DRM_DEV_ERROR(dev, "Failed to set pixel format (%d)\n", ret);<br>
>                goto fail;<br>
> @@ -393,7 +417,6 @@ static int rad_panel_get_modes(struct drm_panel *panel)<br>
>        struct device *dev = &rad->dsi->dev;<br>
>        struct drm_connector *connector = panel->connector;<br>
>        struct drm_display_mode *mode;<br>
> -     u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;<br>
>        u32 *bus_flags = &connector->display_info.bus_flags;<br>
>        int ret;<br>
>  <br>
> @@ -420,7 +443,7 @@ static int rad_panel_get_modes(struct drm_panel *panel)<br>
>                *bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;<br>
>  <br>
>        ret = drm_display_info_set_bus_formats(&connector->display_info,<br>
> -                                            &bus_format, 1);<br>
> +                     rad_bus_formats, ARRAY_SIZE(rad_bus_formats));<br>
>        if (ret)<br>
>                return ret;<br>
>  <br>
> @@ -492,7 +515,7 @@ static const struct drm_panel_funcs rad_panel_funcs = {<br>
>   * to 132MHz (60Hz refresh rate)<br>
>   */<br>
>  static const struct display_timing rad_default_timing = {<br>
> -     .pixelclock = { 66000000, 120000000, 132000000 },<br>
> +     .pixelclock = { 66000000, 132000000, 132000000 },<br>
<br>
This seems like a fix that should be squashed into patch 1.</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">[Robert] I did that to preserve the author, but I can squash it into patch 1 and add her there.<br>
<br>
Thierry<br>
</div>
</span></font></div>
</div>
</div>
</body>
</html>