On Mon, 05 Sep 2022 10:40:58 +0200,
Thomas Zimmermann wrote:
>
> Hi
>
> Am 16.08.22 um 17:36 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@xxxxxxx>
> > ---
> > drivers/gpu/drm/udl/udl_modeset.c | 34 ++++++-------------------
> > drivers/gpu/drm/udl/udl_transfer.c | 40 ------------------------------
> > 2 files changed, 8 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
> > index c34d564773a3..bca31c890108 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)
> > @@ -277,15 +255,19 @@ static int udl_handle_damage(struct drm_framebuffer *fb,
> > struct drm_rect clip;
> > int log_bpp;
> > + if (width <= 0 || height <= 0)
> > + return 0;
> > +
>
> That shouldn't happen.
>
> > ret = udl_log_cpp(fb->format->cpp[0]);
> > if (ret < 0)
> > 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))
> > + clip.x1 = x;
> > + clip.y1 = y;
> > + clip.x2 = x + width;
> > + clip.y2 = y + height;
>
> drm_rect_init() please.
>
> > + if (clip.x2 > fb->width || clip.y2 > fb->height)
>
> That's another thing that should not happen. The damage clips in the
> plane state is what you what to copy. The DRM helpers ensure that
> these various plane, fb and clip coordinates add up.
OK, then we can drop those clip size checks completely.
Will do that in v2 patch.
thanks,
Takashi
