Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> writes:
Hello Geert,
Thanks a lot for your patch, this has been on my TODO for some time!
> The native display format is monochrome light-on-dark (R1).
> Hence add support for R1, so monochrome applications can avoid the
> overhead of back-and-forth conversions between R1 and XR24.
>
> Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> ---
> This work interfered with commit 49d7d581ceaf4cf8 ("drm/ssd130x: Don't
> allocate buffers on each plane update") in drm-misc/for-linux-next,
> which always allocates the buffer upfront, while it is no longer needed
> when never using XR24.
>
you mean R1 here, right ? It's still used in ssd130x_clear_screen() though.
> Probably ssd130x->buffer should be allocated on first use.
Yes, that makes sense.
> And why not allocate the buffers using devm_kcalloc()?
I think there are some lifetimes discrepancies between struct device and
struct drm_device objects. But we could use drm_device managed resources
helpers, i.e: drmm_kzalloc().
> ---
> drivers/gpu/drm/solomon/ssd130x.c | 57 ++++++++++++++++++++++---------
> 1 file changed, 40 insertions(+), 17 deletions(-)
>
[...]
> + case DRM_FORMAT_XRGB8888:
> + dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8);
> + buf = ssd130x->buffer;
> + if (!buf)
> + return 0;
> +
I think this check is not needed anymore now that the driver won't attempt
to update planes for disabled CRTCs ?
It's OK for me to be paranoid though, specially after the other issue that
you found. So I'll let you decide if you think is worth to keep the check.
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
