diff mbox series

[v2,2/8] drm/ssd130x: Fix screen clearing

Message ID c19cd5a57205597bb38a446c3871092993498f01.1692888745.git.geert@linux-m68k.org (mailing list archive)
State New, archived
Headers show
Series drm: fb-helper/ssd130x: Add support for DRM_FORMAT_R1 | expand

Commit Message

Geert Uytterhoeven Aug. 24, 2023, 3:08 p.m. UTC
Due to the reuse of buffers, ssd130x_clear_screen() no longers clears
the screen, but merely redraws the last image that is residing in the
intermediate buffer.

As there is no point in clearing the intermediate buffer and transposing
an all-black image, fix this by just clearing the HW format buffer, and
writing it to the panel.

Fixes: 49d7d581ceaf4cf8 ("drm/ssd130x: Don't allocate buffers on each plane update")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
ssd130x_clear_screen() is only called from
ssd130x_primary_plane_helper_atomic_disable(), but this never happens on
my system.

Tested by adding some extra calls to ssd130x_clear_screen() at regular
intervals.

v2:
  - New.
---
 drivers/gpu/drm/solomon/ssd130x.c | 47 +++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 8 deletions(-)

Comments

Javier Martinez Canillas Aug. 29, 2023, 8:40 a.m. UTC | #1
Geert Uytterhoeven <geert@linux-m68k.org> writes:

Hello Geert,

Thanks a lot for your patch.

> Due to the reuse of buffers, ssd130x_clear_screen() no longers clears
> the screen, but merely redraws the last image that is residing in the
> intermediate buffer.
>
> As there is no point in clearing the intermediate buffer and transposing
> an all-black image, fix this by just clearing the HW format buffer, and
> writing it to the panel.
>
> Fixes: 49d7d581ceaf4cf8 ("drm/ssd130x: Don't allocate buffers on each plane update")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> ssd130x_clear_screen() is only called from
> ssd130x_primary_plane_helper_atomic_disable(), but this never happens on
> my system.
>

AFAIU this should be called if the outputs get disabled.

> Tested by adding some extra calls to ssd130x_clear_screen() at regular
> intervals.
>
> v2:
>   - New.
> ---
>  drivers/gpu/drm/solomon/ssd130x.c | 47 +++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 8 deletions(-)
>

The change makes sense to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Javier Martinez Canillas Aug. 29, 2023, 9:12 p.m. UTC | #2
Javier Martinez Canillas <javierm@redhat.com> writes:

[...]

> The change makes sense to me.
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>

I've also tested this patch now and found no regressions.

Tested-by: Javier Martinez Canillas <javierm@redhat.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 5a80b228d18cae33..78272b1f9d5b167f 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -553,14 +553,45 @@  static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
 static void ssd130x_clear_screen(struct ssd130x_device *ssd130x,
 				 struct ssd130x_plane_state *ssd130x_state)
 {
-	struct drm_rect fullscreen = {
-		.x1 = 0,
-		.x2 = ssd130x->width,
-		.y1 = 0,
-		.y2 = ssd130x->height,
-	};
-
-	ssd130x_update_rect(ssd130x, ssd130x_state, &fullscreen);
+	unsigned int page_height = ssd130x->device_info->page_height;
+	unsigned int pages = DIV_ROUND_UP(ssd130x->height, page_height);
+	u8 *data_array = ssd130x_state->data_array;
+	unsigned int width = ssd130x->width;
+	int ret, i;
+
+	if (!ssd130x->page_address_mode) {
+		memset(data_array, 0, width * pages);
+
+		/* Set address range for horizontal addressing mode */
+		ret = ssd130x_set_col_range(ssd130x, ssd130x->col_offset, width);
+		if (ret < 0)
+			return;
+
+		ret = ssd130x_set_page_range(ssd130x, ssd130x->page_offset, pages);
+		if (ret < 0)
+			return;
+
+		/* Write out update in one go if we aren't using page addressing mode */
+		ssd130x_write_data(ssd130x, data_array, width * pages);
+	} else {
+		/*
+		 * In page addressing mode, the start address needs to be reset,
+		 * and each page then needs to be written out separately.
+		 */
+		memset(data_array, 0, width);
+
+		for (i = 0; i < pages; i++) {
+			ret = ssd130x_set_page_pos(ssd130x,
+						   ssd130x->page_offset + i,
+						   ssd130x->col_offset);
+			if (ret < 0)
+				return;
+
+			ret = ssd130x_write_data(ssd130x, data_array, width);
+			if (ret < 0)
+				return;
+		}
+	}
 }
 
 static int ssd130x_fb_blit_rect(struct drm_plane_state *state,