Re: [PATCH 12/12] drm/format-helper: Move destination-buffer handling into internal helper

[Sam Ravnborg <sam@ravnborg.org>]

Hi Thomas,

> > > -	static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
> > > -		0, 0, 0, 0
> > > -	};
> > >   	const struct drm_format_info *format = fb->format;
> > > -	u8 cpp = format->cpp[0];
> > >   	void (*swab_line)(void *dbuf, const void *sbuf, unsigned int npixels);
> > > -	switch (cpp) {
> > > +	switch (format->cpp[0]) {
> > >   	case 4:
> > >   		swab_line = drm_fb_swab32_line;
> > >   		break;
> > > @@ -230,21 +249,10 @@ void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
> > >   	default:
> > >   		drm_warn_once(fb->dev, "Format %p4cc has unsupported pixel size.\n",
> > >   			      &format->format);
> > > -		swab_line = NULL;
> > > -		break;
> > > -	}
> > > -	if (!swab_line)
> > >   		return;
> > > +	}
> > > -	if (!dst_pitch)
> > > -		dst_pitch = default_dst_pitch;
> > > -
> > > -	if (dst->is_iomem)
> > > -		drm_fb_xfrm_toio(dst[0].vaddr_iomem, dst_pitch[0], cpp,
> > > -				 vmap[0].vaddr, fb, clip, cached, swab_line);
> > > -	else
> > > -		drm_fb_xfrm(dst[0].vaddr, dst_pitch[0], cpp, vmap[0].vaddr, fb,
> > > -			    clip, cached, swab_line);
> > > +	drm_fb_xfrm(dst, dst_pitch, format->cpp, vmap, fb, clip, cached, swab_line);
> > 
> > In this case we pass fb->format-cpp as dst_pitch - so we could retreive
> > is via the fb pointer.
> 
> I don't understand this comment. We're passing format->cpp as dst_pixsize.
> I've meanwhile updated the code to compute the value from
> drm_format_info_bpp().

I wanted to ask if we could drop the format->cpp argument and in
drm_fb_xfrm() use fb->format to retrieve the char per pixel.
So we move this part down in drm_fb_xfrm(), rather than finding
the char per pixel in all callers.

Maybe some of the other callers do not allow this and then this comment
can be ignored.

I have not yet looked at the updated patch-set.

	Sam