Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 08 Sep 2022 09:01:59 +0200
Message-ID: <87v8pycpso.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Thomas Zimmermann <tzimmermann@suse.de>
Cc:     Takashi Iwai <tiwai@suse.de>, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 07/11] drm/udl: Drop unneeded alignment
In-Reply-To: <8291de03-c5b5-e744-62f0-8755c6eada4a@suse.de>
References: <20220906073951.2085-1-tiwai@suse.de>
        <20220906073951.2085-8-tiwai@suse.de>
        <8291de03-c5b5-e744-62f0-8755c6eada4a@suse.de>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

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