[PATCH v3 09/15] drm: tilcdc: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

[Ville Syrjälä]

On Fri, Jun 10, 2016 at 02:51:45PM +0300, 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 at ideasonboard.com>
> > ---
> >  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 23 ++++++++++-------------
> >  1 file changed, 10 insertions(+), 13 deletions(-)
> > 
> > Cc: Jyri Sarha <jsarha at ti.com>
> > 
> > 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.
> 
> >  
> > -	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?

I think what we decided a while ago (at least for i915, but would be good
to use the same convention everywhere) was that ARGB will be assumed to be
pre-multiplied and will enable blending using 1.0*sc+(1.0-sa)*dc as the
function. There have been some efforts at defining some new properties to
control the blend equation, but I guess those got bogged down again.

-- 
Ville Syrjälä
Intel OTC