From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH v3.1 09/15] drm: tilcdc: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp() Date: Fri, 10 Jun 2016 15:21:15 +0300 Message-ID: <575AB0BB.6080407@ti.com> References: <575AA9D1.7090002@ti.com> <1465560488-19974-1-git-send-email-laurent.pinchart@ideasonboard.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0868241951==" Return-path: Received: from comal.ext.ti.com (comal.ext.ti.com [198.47.26.152]) by gabe.freedesktop.org (Postfix) with ESMTPS id 32EA66E0FE for ; Fri, 10 Jun 2016 12:21:21 +0000 (UTC) In-Reply-To: <1465560488-19974-1-git-send-email-laurent.pinchart@ideasonboard.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart , dri-devel@lists.freedesktop.org Cc: "Sarha, Jyri" List-Id: dri-devel@lists.freedesktop.org --===============0868241951== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="d88I8HcKBcHsHFQfFCdBUXWfJrhT0KBpa" --d88I8HcKBcHsHFQfFCdBUXWfJrhT0KBpa Content-Type: multipart/mixed; boundary="1h9IWq80kWiSuMP48wtwe4SD2EM8Vcn2m" From: Tomi Valkeinen To: Laurent Pinchart , dri-devel@lists.freedesktop.org Cc: "Sarha, Jyri" Message-ID: <575AB0BB.6080407@ti.com> Subject: Re: [PATCH v3.1 09/15] drm: tilcdc: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp() References: <575AA9D1.7090002@ti.com> <1465560488-19974-1-git-send-email-laurent.pinchart@ideasonboard.com> In-Reply-To: <1465560488-19974-1-git-send-email-laurent.pinchart@ideasonboard.com> --1h9IWq80kWiSuMP48wtwe4SD2EM8Vcn2m Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/06/16 15:08, Laurent Pinchart wrote: > The driver needs the number of bytes per pixel, not the bpp and depth > info meant for fbdev compatibility. Use the right API. >=20 > In the tilcdc_crtc_mode_set() function compute the hardware register > value directly from the pixel format instead of computing the number of= > bits per pixels first. >=20 > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) >=20 > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/til= cdc/tilcdc_crtc.c > index 79027b1c64d3..1f3f3a1c7b5f 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -65,15 +65,13 @@ static void set_scanout(struct drm_crtc *crtc, stru= ct drm_framebuffer *fb) > struct tilcdc_crtc *tilcdc_crtc =3D to_tilcdc_crtc(crtc); > struct drm_device *dev =3D crtc->dev; > struct drm_gem_cma_object *gem; > - unsigned int depth, bpp; > dma_addr_t start, end; > =20 > - drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp); > gem =3D drm_fb_cma_get_gem_obj(fb, 0); > =20 > start =3D gem->paddr + fb->offsets[0] + > crtc->y * fb->pitches[0] + > - crtc->x * bpp / 8; > + crtc->x * drm_format_plane_cpp(fb->pixel_format, 0); > =20 > end =3D start + (crtc->mode.vdisplay * fb->pitches[0]); > =20 > @@ -132,11 +130,12 @@ static void tilcdc_crtc_destroy(struct drm_crtc *= crtc) > static int tilcdc_verify_fb(struct drm_crtc *crtc, struct drm_framebuf= fer *fb) > { > struct drm_device *dev =3D crtc->dev; > - unsigned int depth, bpp; > + unsigned int pitch; > =20 > - drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp); > + pitch =3D crtc->mode.hdisplay > + * drm_format_plane_cpp(fb->pixel_format, 0); I see you're using this form of having + or * at the start of the continuation line in some patches... I don't like it =3D). And it's not what's being used elsewhere in the driver, afaics. > =20 > - if (fb->pitches[0] !=3D crtc->mode.hdisplay * bpp / 8) { > + if (fb->pitches[0] !=3D pitch) { > dev_err(dev->dev, > "Invalid pitch: fb and crtc widths must be the same"); > return -EINVAL; > @@ -401,16 +400,14 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *= crtc, > if (info->tft_alt_mode) > reg |=3D LCDC_TFT_ALT_ENABLE; > if (priv->rev =3D=3D 2) { > - unsigned int depth, bpp; > - > - drm_fb_get_bpp_depth(crtc->primary->fb->pixel_format, &depth, &bpp);= > - switch (bpp) { > - case 16: > + switch (crtc->primary->fb->pixel_format) { > + case DRM_FORMAT_RGB565: > break; > - case 32: > + case DRM_FORMAT_XRGB8888: > + case DRM_FORMAT_ARGB8888: > reg |=3D LCDC_V2_TFT_24BPP_UNPACK; > /* fallthrough */ > - case 24: > + case DRM_FORMAT_RGB888: > reg |=3D LCDC_V2_TFT_24BPP_MODE; > break; > default: >=20 I think it's better to keep ARGB allowed for now. If we want to disallow it, it's better to be done in a separate patch. Reviewed-by: Tomi Valkeinen Tomi --1h9IWq80kWiSuMP48wtwe4SD2EM8Vcn2m-- --d88I8HcKBcHsHFQfFCdBUXWfJrhT0KBpa Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXWrC7AAoJEPo9qoy8lh717jkP/jN8Pv94uHhcGBWQ549Tup9N ZBwDVlld3f4N9nfeQkDEkVYycE04v4bwrSrOsQV9ur03qWOiIjS931vao0tQQFAS p2zMY+snBvq/p3j5IyJsEKFNO4bwaT6/DX0MF0FF2wijn15rIsVenlIp+UkQrRYx kTOIs7GiE5uhFB0yikJSgSY95/m2y1UUHIj30N3BWgYijigsODTj6Q6MS/x1KpEv aJJbwPDzsENH7b6C6RLQXKhRsAaPiiWxl3dDwFQC8ymVbigdn39lwxjlz1lhNy3L /FJDSLrXQcKCecOSpafmWB/B6LCUVlfZhRadxPcjSEAcGxDixUxESkoaMsFSSlMw dj82ESL4ky4pVJp4VxDcOtOtVMPziaOj4BVdaSKjWhs5Wb4jUhDpJD8Upe2Saqym 0ynEGFMFcgRw/fAktcn/77Zbj/N8WNkBsAF43UsUP9T9Of2OJfeL7o3uN47ptwoy zVgipxC605NoWT/nhuyOpayfLOp1zjin5pneDmFtMaRLtDRGBYtxt0FfliAFlv8B w11ldssSWM3LEDqop2M9IJDcylaz4Olppf4m/Awi1aPizdA/29lFJBvtaGJEoHUK XzttfmUCC88uVV6b0ADA1BqcT7IZPFxA7JV7n8pbs3Ss2XKQAiOhUJp733yWQlkx zCkLt0RnvsKFQDQddz95 =9p21 -----END PGP SIGNATURE----- --d88I8HcKBcHsHFQfFCdBUXWfJrhT0KBpa-- --===============0868241951== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0868241951==--