Re: [PATCH v4 10/14] drm: amdgpu: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v4 10/14] drm: amdgpu: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()
Date: Wed, 21 Sep 2016 15:39:54 +0300	[thread overview]
Message-ID: <2164192.aJ0yNYtrF0@avalon> (raw)
In-Reply-To: <20160921075144.GG4714@dvetter-linux.ger.corp.intel.com>

Hi Daniel,

Thank you for the review.

On Wednesday 21 Sep 2016 09:51:44 Daniel Vetter wrote:
> On Thu, Sep 08, 2016 at 05:44:24PM +0300, 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.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v3:
> > 
> > - Renamed bpp to cpp
> > ---
> > 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  | 14 +++++++-------
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  3 ++-
> >  2 files changed, 9 insertions(+), 8 deletions(-)
> > 
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: Michel Dänzer <michel@daenzer.net>
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c index bf033b58056c..0727946db189
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> > @@ -62,12 +62,12 @@ static struct fb_ops amdgpufb_ops = {
> > 
> >  };
> > 
> > -int amdgpu_align_pitch(struct amdgpu_device *adev, int width, int bpp,
> > bool tiled)
> > +int amdgpu_align_pitch(struct amdgpu_device *adev, int> width, int cpp,
> > bool tiled)
> >  {
> >  	int aligned = width;
> >  	int pitch_mask = 0;
> > 
> > -	switch (bpp / 8) {
> > +	switch (cpp) {
> >  	case 1:
> >  		pitch_mask = 255;
> >  		break;
> > @@ -82,7 +82,7 @@ int amdgpu_align_pitch(struct amdgpu_device *adev, int
> > width, int bpp, bool tile
> >  	aligned += pitch_mask;
> >  	aligned &= ~pitch_mask;
> > 
> > -	return aligned;
> > +	return aligned * cpp;
> 
> Now you multiply by cpp after the rounding.

That's right, but I don't think that's a problem, as all bpp values returned 
by drm_fb_get_bpp_depth() are multiple of 8 bits.

> Otherwise looks reasonable.
> -Daniel
> 
> >  }
> >  
> >  static void amdgpufb_destroy_pinned_object(struct drm_gem_object *gobj)
> > @@ -111,13 +111,13 @@ static int amdgpufb_create_pinned_object(struct
> > amdgpu_fbdev *rfbdev,
> >  	int ret;
> >  	int aligned_size, size;
> >  	int height = mode_cmd->height;
> > -	u32 bpp, depth;
> > +	u32 cpp;
> > 
> > -	drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
> > +	cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
> > 
> >  	/* need to align pitch with crtc limits */
> > 
> > -	mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, bpp,
> > -						  fb_tiled) * ((bpp + 1) / 8);
> > +	mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
> > +						  fb_tiled);
> > 
> >  	height = ALIGN(mode_cmd->height, 8);
> >  	size = mode_cmd->pitches[0] * height;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index
> > 88fbed2389c0..20a4e569b245 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > @@ -704,7 +704,8 @@ int amdgpu_mode_dumb_create(struct drm_file
> > *file_priv,
> >  	uint32_t handle;
> >  	int r;
> > 
> > -	args->pitch = amdgpu_align_pitch(adev, args->width, args->bpp, 0) *
> > ((args->bpp + 1) / 8);
> > +	args->pitch = amdgpu_align_pitch(adev,> args->width,
> > +					 DIV_ROUND_UP(args->bpp, 8), 0);
> > 
> >  	args->size = (u64)args->pitch * args->height;
> >  	args->size = ALIGN(args->size, PAGE_SIZE);

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel