Hi

Am 08.11.21 um 19:57 schrieb Noralf Trønnes:

Den 01.11.2021 15.15, skrev Thomas Zimmermann:

quoted

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;

You're inside drm_dev_enter here so can't just return. Move
drm_dev_enter after this like you do in update().

Right. Will be fixed.

quoted

+
+	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;
+

The following check, isn't that the same check that has just happened in
drm_atomic_helper_damage_iter_next()?

Oh, indeed. It takes the plane's src rectangle [1] and clips against it. 
[2] The bounding box of all damage clips is the final rectangle. [3] It 
can never by outside of plane_state->src. So the extra test here and the 
damage_clip variable can be removed.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_damage_helper.c#L235
[2] 
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_damage_helper.c#L280
[3] 
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_damage_helper.c#L320

Noralf.

quoted

+	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