diff mbox series

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

Message ID 20220906073951.2085-8-tiwai@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/udl: More fixes | expand

Commit Message

Takashi Iwai Sept. 6, 2022, 7:39 a.m. UTC 
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>
---
 drivers/gpu/drm/udl/udl_modeset.c  | 28 +--------------------
 drivers/gpu/drm/udl/udl_transfer.c | 40 ------------------------------
 2 files changed, 1 insertion(+), 67 deletions(-)

Comments

Thomas Zimmermann Sept. 7, 2022, 7:29 a.m. UTC | #1 
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.

Best regards
Thomas

>   
>   	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
>   	if (ret)
> diff --git a/drivers/gpu/drm/udl/udl_transfer.c b/drivers/gpu/drm/udl/udl_transfer.c
> index 176ef2a6a731..a431208dda85 100644
> --- a/drivers/gpu/drm/udl/udl_transfer.c
> +++ b/drivers/gpu/drm/udl/udl_transfer.c
> @@ -25,46 +25,6 @@
>   #define MIN_RAW_PIX_BYTES	2
>   #define MIN_RAW_CMD_BYTES	(RAW_HEADER_BYTES + MIN_RAW_PIX_BYTES)
>   
> -/*
> - * Trims identical data from front and back of line
> - * Sets new front buffer address and width
> - * And returns byte count of identical pixels
> - * Assumes CPU natural alignment (unsigned long)
> - * for back and front buffer ptrs and width
> - */
> -#if 0
> -static int udl_trim_hline(const u8 *bback, const u8 **bfront, int *width_bytes)
> -{
> -	int j, k;
> -	const unsigned long *back = (const unsigned long *) bback;
> -	const unsigned long *front = (const unsigned long *) *bfront;
> -	const int width = *width_bytes / sizeof(unsigned long);
> -	int identical = width;
> -	int start = width;
> -	int end = width;
> -
> -	for (j = 0; j < width; j++) {
> -		if (back[j] != front[j]) {
> -			start = j;
> -			break;
> -		}
> -	}
> -
> -	for (k = width - 1; k > j; k--) {
> -		if (back[k] != front[k]) {
> -			end = k+1;
> -			break;
> -		}
> -	}
> -
> -	identical = start + (width - end);
> -	*bfront = (u8 *) &front[start];
> -	*width_bytes = (end - start) * sizeof(unsigned long);
> -
> -	return identical * sizeof(unsigned long);
> -}
> -#endif
> -
>   static inline u16 pixel32_to_be16(const uint32_t pixel)
>   {
>   	return (((pixel >> 3) & 0x001f) |
Takashi Iwai Sept. 8, 2022, 7:01 a.m. UTC | #2 
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
diff mbox series

Patch

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);
 
 	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
 	if (ret)
diff --git a/drivers/gpu/drm/udl/udl_transfer.c b/drivers/gpu/drm/udl/udl_transfer.c
index 176ef2a6a731..a431208dda85 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -25,46 +25,6 @@ 
 #define MIN_RAW_PIX_BYTES	2
 #define MIN_RAW_CMD_BYTES	(RAW_HEADER_BYTES + MIN_RAW_PIX_BYTES)
 
-/*
- * Trims identical data from front and back of line
- * Sets new front buffer address and width
- * And returns byte count of identical pixels
- * Assumes CPU natural alignment (unsigned long)
- * for back and front buffer ptrs and width
- */
-#if 0
-static int udl_trim_hline(const u8 *bback, const u8 **bfront, int *width_bytes)
-{
-	int j, k;
-	const unsigned long *back = (const unsigned long *) bback;
-	const unsigned long *front = (const unsigned long *) *bfront;
-	const int width = *width_bytes / sizeof(unsigned long);
-	int identical = width;
-	int start = width;
-	int end = width;
-
-	for (j = 0; j < width; j++) {
-		if (back[j] != front[j]) {
-			start = j;
-			break;
-		}
-	}
-
-	for (k = width - 1; k > j; k--) {
-		if (back[k] != front[k]) {
-			end = k+1;
-			break;
-		}
-	}
-
-	identical = start + (width - end);
-	*bfront = (u8 *) &front[start];
-	*width_bytes = (end - start) * sizeof(unsigned long);
-
-	return identical * sizeof(unsigned long);
-}
-#endif
-
 static inline u16 pixel32_to_be16(const uint32_t pixel)
 {
 	return (((pixel >> 3) & 0x001f) |