Re: [PATCH v2 07/11] drm/udl: Drop unneeded alignment

[Takashi Iwai <tiwai@suse.de>]

On Wed, 07 Sep 2022 09:29:37 +0200,
Thomas Zimmermann wrote:
> 
> Hi
> 
> Am 06.09.22 um 09:39 schrieb Takashi Iwai:
> > The alignment of damaged area was needed for the original udlfb driver
> > that tried to trim the superfluous copies between front and backend
> > buffers and handle data in long int.  It's not the case for udl DRM
> > driver, hence we can omit the whole unneeded alignment, as well as the
> > dead code.
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> with an entirely optional comment below.
> 
> > ---
> >   drivers/gpu/drm/udl/udl_modeset.c  | 28 +--------------------
> >   drivers/gpu/drm/udl/udl_transfer.c | 40 ------------------------------
> >   2 files changed, 1 insertion(+), 67 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
> > index c34d564773a3..9896c16c74f5 100644
> > --- a/drivers/gpu/drm/udl/udl_modeset.c
> > +++ b/drivers/gpu/drm/udl/udl_modeset.c
> > @@ -243,28 +243,6 @@ static long udl_log_cpp(unsigned int cpp)
> >   	return __ffs(cpp);
> >   }
> >   -static int udl_aligned_damage_clip(struct drm_rect *clip, int x,
> > int y,
> > -				   int width, int height)
> > -{
> > -	int x1, x2;
> > -
> > -	if (WARN_ON_ONCE(x < 0) ||
> > -	    WARN_ON_ONCE(y < 0) ||
> > -	    WARN_ON_ONCE(width < 0) ||
> > -	    WARN_ON_ONCE(height < 0))
> > -		return -EINVAL;
> > -
> > -	x1 = ALIGN_DOWN(x, sizeof(unsigned long));
> > -	x2 = ALIGN(width + (x - x1), sizeof(unsigned long)) + x1;
> > -
> > -	clip->x1 = x1;
> > -	clip->y1 = y;
> > -	clip->x2 = x2;
> > -	clip->y2 = y + height;
> > -
> > -	return 0;
> > -}
> > -
> >   static int udl_handle_damage(struct drm_framebuffer *fb,
> >   			     const struct iosys_map *map,
> >   			     int x, int y, int width, int height)
> > @@ -282,11 +260,7 @@ static int udl_handle_damage(struct drm_framebuffer *fb,
> >   		return ret;
> >   	log_bpp = ret;
> >   -	ret = udl_aligned_damage_clip(&clip, x, y, width, height);
> > -	if (ret)
> > -		return ret;
> > -	else if ((clip.x2 > fb->width) || (clip.y2 > fb->height))
> > -		return -EINVAL;
> > +	drm_rect_init(&clip, x, y, width, height);
> 
> The clip rectangle could be passed directly by the caller, which would
> simplify the update function. But that's really just nitpicking.

OK, will add a patch to do this, too :)


thanks,

Takashi