| Message ID | 1465560488-19974-1-git-send-email-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > > 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. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/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, struct drm_framebuffer *fb) > struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); > struct drm_device *dev = crtc->dev; > struct drm_gem_cma_object *gem; > - unsigned int depth, bpp; > dma_addr_t start, end; > > - drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp); > gem = drm_fb_cma_get_gem_obj(fb, 0); > > start = gem->paddr + fb->offsets[0] + > crtc->y * fb->pitches[0] + > - crtc->x * bpp / 8; > + crtc->x * drm_format_plane_cpp(fb->pixel_format, 0); > > end = start + (crtc->mode.vdisplay * fb->pitches[0]); > > @@ -132,11 +130,12 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc) > static int tilcdc_verify_fb(struct drm_crtc *crtc, struct drm_framebuffer *fb) > { > struct drm_device *dev = crtc->dev; > - unsigned int depth, bpp; > + unsigned int pitch; > > - drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp); > + pitch = 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 =). And it's not what's being used elsewhere in the driver, afaics. > > - if (fb->pitches[0] != crtc->mode.hdisplay * bpp / 8) { > + if (fb->pitches[0] != 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 |= LCDC_TFT_ALT_ENABLE; > if (priv->rev == 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 |= LCDC_V2_TFT_24BPP_UNPACK; > /* fallthrough */ > - case 24: > + case DRM_FORMAT_RGB888: > reg |= LCDC_V2_TFT_24BPP_MODE; > break; > default: > 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.valkeinen@ti.com> Tomi
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/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, struct drm_framebuffer *fb) struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc); struct drm_device *dev = crtc->dev; struct drm_gem_cma_object *gem; - unsigned int depth, bpp; dma_addr_t start, end; - drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp); gem = drm_fb_cma_get_gem_obj(fb, 0); start = gem->paddr + fb->offsets[0] + crtc->y * fb->pitches[0] + - crtc->x * bpp / 8; + crtc->x * drm_format_plane_cpp(fb->pixel_format, 0); end = start + (crtc->mode.vdisplay * fb->pitches[0]); @@ -132,11 +130,12 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc) static int tilcdc_verify_fb(struct drm_crtc *crtc, struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; - unsigned int depth, bpp; + unsigned int pitch; - drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp); + pitch = crtc->mode.hdisplay + * drm_format_plane_cpp(fb->pixel_format, 0); - if (fb->pitches[0] != crtc->mode.hdisplay * bpp / 8) { + if (fb->pitches[0] != 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 |= LCDC_TFT_ALT_ENABLE; if (priv->rev == 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 |= LCDC_V2_TFT_24BPP_UNPACK; /* fallthrough */ - case 24: + case DRM_FORMAT_RGB888: reg |= LCDC_V2_TFT_24BPP_MODE; break; default:
The driver needs the number of bytes per pixel, not the bpp and depth info meant for fbdev compatibility. Use the right API. 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. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)