<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>