From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 2/2] drm/panel: rm67191: Add support for new bus formats Date: Fri, 27 Apr 2018 13:10:10 +0200 Message-ID: <20180427111010.GB21066@ulmo> References: <1522751401-3381-1-git-send-email-robert.chiras@nxp.com> <1522751401-3381-3-git-send-email-robert.chiras@nxp.com> <20180426145611.GE31888@ulmo> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0170766345==" Return-path: Received: from mail-wr0-x244.google.com (mail-wr0-x244.google.com [IPv6:2a00:1450:400c:c0c::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4CB5E6E8BC for ; Fri, 27 Apr 2018 11:10:13 +0000 (UTC) Received: by mail-wr0-x244.google.com with SMTP id v5-v6so1384937wrf.9 for ; Fri, 27 Apr 2018 04:10:13 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Robert Chiras Cc: dl-linux-imx , "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org --===============0170766345== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="cvVnyQ+4j833TQvp" Content-Disposition: inline --cvVnyQ+4j833TQvp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Apr 27, 2018 at 10:38:24AM +0000, Robert Chiras wrote: >=20 >=20 >=20 > ________________________________ > From: Thierry Reding > Sent: Thursday, April 26, 2018 5:56 PM > To: Robert Chiras > Cc: dl-linux-imx; dri-devel@lists.freedesktop.org > Subject: Re: [PATCH 2/2] drm/panel: rm67191: Add support for new bus form= ats >=20 > On Tue, Apr 03, 2018 at 01:30:01PM +0300, Robert Chiras wrote: > > From: Mirela Rabulea > > > > Do not hardcode pixel_format to 0x77 but calculate it from dsi->format. > > Report all the supported bus formats in get_modes: > > MEDIA_BUS_FMT_RGB888_1X24 > > MEDIA_BUS_FMT_RGB666_1X18 > > MEDIA_BUS_FMT_RGB565_1X16 > > Change pixelclock from 120 to 132 MHz, or 16 bpp formats will not work. > > > > Signed-off-by: Mirela Rabulea > > --- > > drivers/gpu/drm/panel/panel-raydium-rm67191.c | 33 +++++++++++++++++++= ++++---- > > 1 file changed, 28 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gp= u/drm/panel/panel-raydium-rm67191.c > > index 07b0bd4..6331fef 100644 > > --- a/drivers/gpu/drm/panel/panel-raydium-rm67191.c > > +++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c > > @@ -178,6 +178,12 @@ static const cmd_set_table manufacturer_cmd_set[] = =3D { > > {0x51, 0x04}, > > }; > > > > +static const u32 rad_bus_formats[] =3D { > > + MEDIA_BUS_FMT_RGB888_1X24, > > + MEDIA_BUS_FMT_RGB666_1X18, > > + MEDIA_BUS_FMT_RGB565_1X16, > > +}; > > + > > struct rad_panel { > > struct drm_panel base; > > struct mipi_dsi_device *dsi; > > @@ -215,11 +221,27 @@ static int rad_panel_push_cmd_list(struct mipi_ds= i_device *dsi) > > return ret; > > }; > > > > +static int color_format_from_dsi_format(enum mipi_dsi_pixel_format for= mat) > > +{ > > + switch (format) { > > + case MIPI_DSI_FMT_RGB565: > > + return 0x55; > > + case MIPI_DSI_FMT_RGB666: > > + case MIPI_DSI_FMT_RGB666_PACKED: > > + return 0x66; > > + case MIPI_DSI_FMT_RGB888: > > + return 0x77; > > + default: > > + return 0x77; /* for backward compatibility */ > > + } > > +}; > > + > > static int rad_panel_prepare(struct drm_panel *panel) > > { > > struct rad_panel *rad =3D to_rad_panel(panel); > > struct mipi_dsi_device *dsi =3D rad->dsi; > > struct device *dev =3D &dsi->dev; > > + int color_format =3D color_format_from_dsi_format(dsi->format); > > int ret; > > > > if (rad->prepared) > > @@ -276,8 +298,10 @@ static int rad_panel_prepare(struct drm_panel *pan= el) > > DRM_DEV_ERROR(dev, "Failed to set tear scanline (%d)\n",= ret); > > goto fail; > > } > > - /* Set pixel format to RGB888 */ > > - ret =3D mipi_dsi_dcs_set_pixel_format(dsi, 0x77); > > + /* Set pixel format */ > > + ret =3D mipi_dsi_dcs_set_pixel_format(dsi, color_format); > > + DRM_DEV_DEBUG_DRIVER(dev, "Interface color format set to 0x%x\n", > > + color_format); > > if (ret < 0) { > > DRM_DEV_ERROR(dev, "Failed to set pixel format (%d)\n", = ret); > > goto fail; > > @@ -393,7 +417,6 @@ static int rad_panel_get_modes(struct drm_panel *pa= nel) > > struct device *dev =3D &rad->dsi->dev; > > struct drm_connector *connector =3D panel->connector; > > struct drm_display_mode *mode; > > - u32 bus_format =3D MEDIA_BUS_FMT_RGB888_1X24; > > u32 *bus_flags =3D &connector->display_info.bus_flags; > > int ret; > > > > @@ -420,7 +443,7 @@ static int rad_panel_get_modes(struct drm_panel *pa= nel) > > *bus_flags |=3D DRM_BUS_FLAG_PIXDATA_POSEDGE; > > > > ret =3D drm_display_info_set_bus_formats(&connector->display_inf= o, > > - &bus_format, 1); > > + rad_bus_formats, ARRAY_SIZE(rad_bus_formats)); > > if (ret) > > return ret; > > > > @@ -492,7 +515,7 @@ static const struct drm_panel_funcs rad_panel_funcs= =3D { > > * to 132MHz (60Hz refresh rate) > > */ > > static const struct display_timing rad_default_timing =3D { > > - .pixelclock =3D { 66000000, 120000000, 132000000 }, > > + .pixelclock =3D { 66000000, 132000000, 132000000 }, >=20 > This seems like a fix that should be squashed into patch 1. >=20 > [Robert] I did that to preserve the author, but I can squash it into > patch 1 and add her there. If you want to preserve authorship, just split it off into a separate patch. We don't usually do that for initial submissions of a driver, so you can usually squash such subsequent fixes into the initial patch for an initial submission and mention contributors in the commit message. I don't have any strong objections if you want to split the patch out separately, though. Thierry --cvVnyQ+4j833TQvp Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAlrjBREACgkQ3SOs138+ s6Ha9g//fM68qBEHWoPhc/kfm4m6oZVWCOmVw94WKnPyAaoJ+RKYJ6dNSLq85dWq x5r3MWsAVK3wa7DgpfJHJUwZi3XrB3vyMd881XIrGo5VF7mmn6R31F8JbbbKhRnx n2ygAhvbGHjNsNA+GYnPTtcFMn57zf+n2qgivlFQcSPdnxCPKdxilXyHPyFfnCRF RLqZFOXzxauu7a4TNVdF7VyLm8IwIkLZwBOMSSUuQqxQiW3P62CHZug0tcvFvP24 lz65J9jYANaGpgtD63XHF1X39abqKFnHlj5WGN9hO5kpzJeEufZ6t9uH8gUeZXQn KIu9qEPPPimr62rNXCZcaVV7euAeUmmUc5Tr36xpgw0zFSyZH2FFqMHlMJII+dVE K+DdKe2Msw7IruW+CBG/YiOwkLNdOxXkeWhUfBzQ52Ig0ztNRfjAJmihBdKUgRky maoQg+5I98q19N/x63MZ0/2GtoiKQHbeO5c67LeZvEky9T8/QrkMs7TcjIRJ67Az JoMeTohyDQiR00gcAR5ohFOAnWB25ep6JFCMtxgfwKG7bw7AKjOnEib589d+fn55 FBK4RaaQpz2/iTrJj8FvmqzEgBfJess0Z39WPzITuL8Jb/uq8hiqQ5KnB4vqwvF1 ai/d0zs/Gsb42tE6yFlCHF4xwRm1Q+ep1g0xJp7UY51V+wu+oCg= =Iygk -----END PGP SIGNATURE----- --cvVnyQ+4j833TQvp-- --===============0170766345== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0170766345==--