Hi Tomi,
On Friday 10 Jun 2016 14:51:45 Tomi Valkeinen wrote:
> On 09/06/16 02:32, 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@xxxxxxxxxxxxxxxx>
> > ---
> >
> > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 23 ++++++++++-------------
> > 1 file changed, 10 insertions(+), 13 deletions(-)
> >
> > Cc: Jyri Sarha <jsarha@xxxxxx>
> >
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> > b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 79027b1c64d3..d63c7363dabc
> > 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 min_pitch;
> >
> > - drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
> > + min_pitch = crtc->mode.hdisplay
> > + * drm_format_plane_cpp(fb->pixel_format, 0);
>
> Why 'min_pitch'? Plain 'pitch' should be fine here.
You're right, the hardware doesn't seem to support configurable strides, I'll
rename this.
> > - if (fb->pitches[0] != crtc->mode.hdisplay * bpp / 8) {
> > + if (fb->pitches[0] != min_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:
>
> I'm not sure what's the common way, but tilcdc doesn't support alpha.
> ARGB works, of course, by ignoring A, but... If an userspace app creates
> ARGB buffer, does the app expect alpha to work?
The driver currently accepts DRM_FORMAT_ARGB8888 and ignores the alpha
component. I didn't want to risk breaking userspace by removing that, so I
decided to take a conservative approach. If you think we can safely drop
DRM_FORMAT_ARGB8888 then we can do so in a separate patch.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
