Noralf Trønnes
2021-Nov-08 20:55 UTC
[PATCH v2 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property
Den 01.11.2021 15.15, skrev Thomas Zimmermann:> Enable the FB_DAMAGE_CLIPS property to reduce display-update > overhead. Also fixes a warning in the kernel log. > > simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called > > Fix the computation of the blit rectangle. This wasn't an issue so > far, as simpledrm always blitted the full framebuffer. The code now > supports damage clipping and virtual screen sizes. > > Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de> > --- > drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++-------- > 1 file changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c > index 571f716ff427..e872121e9fb0 100644 > --- a/drivers/gpu/drm/tiny/simpledrm.c > +++ b/drivers/gpu/drm/tiny/simpledrm.c > @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, > void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */ > struct drm_device *dev = &sdev->dev; > void __iomem *dst = sdev->screen_base; > - struct drm_rect clip; > + struct drm_rect src_clip, dst_clip; > int idx; > > if (!fb) > @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, > if (!drm_dev_enter(dev, &idx)) > return; > > - drm_rect_init(&clip, 0, 0, fb->width, fb->height); > + drm_rect_fp_to_int(&src_clip, &plane_state->src); > > - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); > - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); > + dst_clip = plane_state->dst;I assume that src_clip and dst_clip are of the same size, since scaling is not supported. What prevents dst_clip from being bigger than the buffer that's being blitted into? Where is that bounds checking done? Noralf.> + if (!drm_rect_intersect(&dst_clip, &src_clip)) > + return; > + > + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); > + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip); > > drm_dev_exit(idx); > } > @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, > struct drm_framebuffer *fb = plane_state->fb; > struct drm_device *dev = &sdev->dev; > void __iomem *dst = sdev->screen_base; > - struct drm_rect clip; > + struct drm_rect damage_clip, src_clip, dst_clip; > int idx; > > if (!fb) > return; > > - if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip)) > + if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip)) > + return; > + > + drm_rect_fp_to_int(&src_clip, &plane_state->src); > + if (!drm_rect_intersect(&src_clip, &damage_clip)) > + return; > + > + dst_clip = plane_state->dst; > + if (!drm_rect_intersect(&dst_clip, &src_clip)) > return; > > if (!drm_dev_enter(dev, &idx)) > return; > > - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); > - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); > + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); > + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip); > > drm_dev_exit(idx); > } > @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) > if (ret) > return ret; > > + drm_plane_enable_fb_damage_clips(&pipe->plane); > + > drm_mode_config_reset(dev); > > return 0; >
Thomas Zimmermann
2021-Nov-09 12:38 UTC
[PATCH v2 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property
Am 08.11.21 um 21:55 schrieb Noralf Tr?nnes:> > > Den 01.11.2021 15.15, skrev Thomas Zimmermann: >> Enable the FB_DAMAGE_CLIPS property to reduce display-update >> overhead. Also fixes a warning in the kernel log. >> >> simple-framebuffer simple-framebuffer.0: [drm] drm_plane_enable_fb_damage_clips() not called >> >> Fix the computation of the blit rectangle. This wasn't an issue so >> far, as simpledrm always blitted the full framebuffer. The code now >> supports damage clipping and virtual screen sizes. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de> >> --- >> drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++-------- >> 1 file changed, 22 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c >> index 571f716ff427..e872121e9fb0 100644 >> --- a/drivers/gpu/drm/tiny/simpledrm.c >> +++ b/drivers/gpu/drm/tiny/simpledrm.c >> @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, >> void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping abstraction */ >> struct drm_device *dev = &sdev->dev; >> void __iomem *dst = sdev->screen_base; >> - struct drm_rect clip; >> + struct drm_rect src_clip, dst_clip; >> int idx; >> >> if (!fb) >> @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, >> if (!drm_dev_enter(dev, &idx)) >> return; >> >> - drm_rect_init(&clip, 0, 0, fb->width, fb->height); >> + drm_rect_fp_to_int(&src_clip, &plane_state->src); >> >> - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); >> - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); >> + dst_clip = plane_state->dst; > > I assume that src_clip and dst_clip are of the same size, since scaling > is not supported. What prevents dst_clip from being bigger than the > buffer that's being blitted into? Where is that bounds checking done?The value of dst_clip comes from plane_state->dst, which gets initialized in drm_atomic_helper_check_plane_state(). [1] The fields are taken from the crtc_{x,y,w,h} values by drm_plane_get_dest(). [2] For primary planes, the crtc_{x,y,w,h} values are initialized in __drm_atomic_helper_set_config() to the size of the display. [3] That size comes directly from the current video mode. [4] From all I can see this cannot overflow. Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_atomic_helper.c#L807 [2] https://elixir.bootlin.com/linux/latest/source/include/drm/drm_plane.h#L257 [3] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_atomic.c#L1590 [4] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_modes.c#L788> > Noralf. > >> + if (!drm_rect_intersect(&dst_clip, &src_clip)) >> + return; >> + >> + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); >> + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip); >> >> drm_dev_exit(idx); >> } >> @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, >> struct drm_framebuffer *fb = plane_state->fb; >> struct drm_device *dev = &sdev->dev; >> void __iomem *dst = sdev->screen_base; >> - struct drm_rect clip; >> + struct drm_rect damage_clip, src_clip, dst_clip; >> int idx; >> >> if (!fb) >> return; >> >> - if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &clip)) >> + if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage_clip)) >> + return; >> + >> + drm_rect_fp_to_int(&src_clip, &plane_state->src); >> + if (!drm_rect_intersect(&src_clip, &damage_clip)) >> + return; >> + >> + dst_clip = plane_state->dst; >> + if (!drm_rect_intersect(&dst_clip, &src_clip)) >> return; >> >> if (!drm_dev_enter(dev, &idx)) >> return; >> >> - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); >> - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &clip); >> + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); >> + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, &src_clip); >> >> drm_dev_exit(idx); >> } >> @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) >> if (ret) >> return ret; >> >> + drm_plane_enable_fb_damage_clips(&pipe->plane); >> + >> drm_mode_config_reset(dev); >> >> return 0; >>-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 N?rnberg, Germany (HRB 36809, AG N?rnberg) Gesch?ftsf?hrer: Ivo Totev -------------- next part -------------- A non-text attachment was scrubbed... Name: OpenPGP_signature Type: application/pgp-signature Size: 840 bytes Desc: OpenPGP digital signature URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20211109/14f5e830/attachment.sig>