| Message ID | 20211022132829.7697-8-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/simpledrm: Enable damage clips and virtual screens | expand |
Den 22.10.2021 15.28, 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@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; > + 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; > + I'm afraid I don't understand what's going on here, but isn't it possible to put this logic into drm_atomic_helper_damage_merged()? Noralf. > + 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; >
Hi Am 24.10.21 um 17:20 schrieb Noralf Trønnes: > > > Den 22.10.2021 15.28, 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@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; >> + 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; >> + > > I'm afraid I don't understand what's going on here, but isn't it > possible to put this logic into drm_atomic_helper_damage_merged()? The code above gets the damage rectangle (i.e., the plane's area that needs to be updated). The code below get's the framebuffer area. If they don't overlap, return. (I think this can really only fail with the next patch in the series, which adds virtual screens.) > > Noralf. > >> + 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; And here we check if the updated plane/framebuffer area is actually visible on screen; otherwise return. It could be in an area that is off-screen. (Again, this probably only fails with the virtual-screen patch.) I don't think this is all generic enough to be within drm_atomic_helper_damage_merged(). But once we have multiple SHMEM-based drivers with virtual screens, we can move it into a helper for shadow-buffered planes. Your gud driver would be a candidate for this feature as well. Best regards Thomas >> >> 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; >>
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; + 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;
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@suse.de> --- drivers/gpu/drm/tiny/simpledrm.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)