[PATCH v4 03/14] drm: Use drm_format_info() in DRM core code

[Daniel Vetter]

On Thu, Sep 15, 2016 at 01:31:23AM +0300, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the review.
> 
> On Wednesday 14 Sep 2016 16:23:09 Tomi Valkeinen wrote:
> > On 08/09/16 17:44, Laurent Pinchart wrote:
> > > Replace calls to the drm_format_*() helper functions with direct use of
> > > the drm_format_info structure. This improves efficiency by removing
> > > duplicate lookups.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > > 
> > >  drivers/gpu/drm/drm_fb_cma_helper.c |  23 ++++----
> > >  drivers/gpu/drm/drm_framebuffer.c   | 102 ++++---------------------------
> > >  2 files changed, 25 insertions(+), 100 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > b/drivers/gpu/drm/drm_fb_cma_helper.c index 1fd6eac1400c..fac4f06f8485
> > > 100644
> > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> 
> [snip]
> 
> > > @@ -269,12 +269,15 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
> > > 
> > >  static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct
> > >  seq_file *m) {
> > >  	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> > > -	int i, n = drm_format_num_planes(fb->pixel_format);
> > > +	const struct drm_format_info *info;
> > > +	int i;
> > > 
> > >  	seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
> > >  			(char *)&fb->pixel_format);
> > > 
> > > -	for (i = 0; i < n; i++) {
> > > +	info = drm_format_info(fb->pixel_format);
> > > +
> > > +	for (i = 0; i < info->num_planes; i++) {
> > >  		seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
> > >  				i, fb->offsets[i], fb->pitches[i]);
> > >  		drm_gem_cma_describe(fb_cma->obj[i], m);
> > 
> > This change doesn't seem to improve the function. Afaics, only the num
> > planes is retrieved and used.
> 
> You're right. I was actually trying to remove usage of all the wrappers from 
> the DRM core code. If that's not desired I can drop this hunk.

No preference from my side, either way:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch