Messages in this thread Patch in this message | | | Date | Thu, 31 Aug 2023 10:01:46 +0200 | Subject | Re: [PATCH v2 4/8] drm/ssd130x: Add support for DRM_FORMAT_R1 | From | Thomas Zimmermann <> |
| |
Hi
Am 24.08.23 um 17:08 schrieb Geert Uytterhoeven: > The native display format is monochrome light-on-dark (R1). > Hence add support for R1, so monochrome applications not only look > better, but also avoid the overhead of back-and-forth conversions > between R1 and XR24. > > Do not allocate the intermediate conversion buffer when it is not > needed, and reorder the two buffer allocations to streamline operation. > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > v2: > - Rework on top op commit 8c3926367ac9df6c ("drm/ssd130x: Use > shadow-buffer helpers when managing plane's state") in drm/drm-next. > Hence I did not add Javier's tags given on v1. > - Do not allocate intermediate buffer when not needed. > --- > drivers/gpu/drm/solomon/ssd130x.c | 76 +++++++++++++++++++++---------- > 1 file changed, 51 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c > index 78272b1f9d5b167f..18007cb4f3de5aa1 100644 > --- a/drivers/gpu/drm/solomon/ssd130x.c > +++ b/drivers/gpu/drm/solomon/ssd130x.c > @@ -449,15 +449,14 @@ static int ssd130x_init(struct ssd130x_device *ssd130x) > > static int ssd130x_update_rect(struct ssd130x_device *ssd130x, > struct ssd130x_plane_state *ssd130x_state, > + u8 *buf, unsigned int pitch, > struct drm_rect *rect) > { > unsigned int x = rect->x1; > unsigned int y = rect->y1; > - u8 *buf = ssd130x_state->buffer; > u8 *data_array = ssd130x_state->data_array; > unsigned int width = drm_rect_width(rect); > unsigned int height = drm_rect_height(rect); > - unsigned int line_length = DIV_ROUND_UP(width, 8); > unsigned int page_height = ssd130x->device_info->page_height; > unsigned int pages = DIV_ROUND_UP(height, page_height); > struct drm_device *drm = &ssd130x->drm; > @@ -516,7 +515,7 @@ static int ssd130x_update_rect(struct ssd130x_device *ssd130x, > u8 data = 0; > > for (k = 0; k < m; k++) { > - u8 byte = buf[(8 * i + k) * line_length + j / 8]; > + u8 byte = buf[(8 * i + k) * pitch + j / 8]; > u8 bit = (byte >> (j % 8)) & 1; > > data |= bit << k; > @@ -602,27 +601,51 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
The handing of different formats in this function is confusing. I'd rather implement ssd130x_fb_blit_rect_r1 and ssd130x_fb_blit_rect_xrgb8888 which then handle a single format.
> struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev); > unsigned int page_height = ssd130x->device_info->page_height; > struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(state); > - u8 *buf = ssd130x_state->buffer; > struct iosys_map dst; > unsigned int dst_pitch; > int ret = 0; > + u8 *buf; > > /* Align y to display page boundaries */ > rect->y1 = round_down(rect->y1, page_height); > rect->y2 = min_t(unsigned int, round_up(rect->y2, page_height), ssd130x->height); > > - dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8); > + switch (fb->format->format) { > + case DRM_FORMAT_R1: > + /* Align x to byte boundaries */ > + rect->x1 = round_down(rect->x1, 8); > + rect->x2 = round_up(rect->x2, 8);
Is rounding to multiples of 8 guaranteed to work? I know it did on VGA-compatible HW, because VGA enforces it. But is that generally the case?
> > - ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE); > - if (ret) > - return ret; > + ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE); > + if (ret) > + return ret; > + > + dst_pitch = fb->pitches[0]; > + buf = vmap[0].vaddr + rect->y1 * dst_pitch + rect->x1 / 8; > + > + ssd130x_update_rect(ssd130x, ssd130x_state, buf, dst_pitch, > + rect); > + > + drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE); > + break; > + > + case DRM_FORMAT_XRGB8888: > + dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8); > + buf = ssd130x_state->buffer; > + > + ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE); > + if (ret) > + return ret; > > - iosys_map_set_vaddr(&dst, buf); > - drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect); > + iosys_map_set_vaddr(&dst, buf); > + drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect); > > - drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE); > + drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE); > > - ssd130x_update_rect(ssd130x, ssd130x_state, rect); > + ssd130x_update_rect(ssd130x, ssd130x_state, buf, dst_pitch, > + rect); > + break; > + } > > return ret; > } > @@ -644,22 +667,24 @@ static int ssd130x_primary_plane_helper_atomic_check(struct drm_plane *plane, > if (ret) > return ret; > > - fi = drm_format_info(DRM_FORMAT_R1); > - if (!fi) > - return -EINVAL; > + ssd130x_state->data_array = kcalloc(ssd130x->width, pages, GFP_KERNEL); > + if (!ssd130x_state->data_array) > + return -ENOMEM; > > - pitch = drm_format_info_min_pitch(fi, 0, ssd130x->width); > + if (plane_state->fb->format->format != DRM_FORMAT_R1) { > + fi = drm_format_info(DRM_FORMAT_R1); > + if (!fi) > + return -EINVAL; > > - ssd130x_state->buffer = kcalloc(pitch, ssd130x->height, GFP_KERNEL); > - if (!ssd130x_state->buffer) > - return -ENOMEM; > + pitch = drm_format_info_min_pitch(fi, 0, ssd130x->width); > > - ssd130x_state->data_array = kcalloc(ssd130x->width, pages, GFP_KERNEL); > - if (!ssd130x_state->data_array) { > - kfree(ssd130x_state->buffer); > - /* Set to prevent a double free in .atomic_destroy_state() */ > - ssd130x_state->buffer = NULL; > - return -ENOMEM; > + ssd130x_state->buffer = kcalloc(pitch, ssd130x->height, GFP_KERNEL); > + if (!ssd130x_state->buffer) { > + kfree(ssd130x_state->data_array); > + /* Set to prevent a double free in .atomic_destroy_state() */ > + ssd130x_state->data_array = NULL; > + return -ENOMEM; > + } > } > > return 0; > @@ -898,6 +923,7 @@ static const struct drm_mode_config_funcs ssd130x_mode_config_funcs = { > }; > > static const uint32_t ssd130x_formats[] = { > + DRM_FORMAT_R1, > DRM_FORMAT_XRGB8888, > }; >
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) [unhandled content-type:application/pgp-signature] | |