Re: [PATCH 4/8] drm/ssd130x: Add support for DRM_FORMAT_R1

[Geert Uytterhoeven]

Hi Javier,

On Fri, Jul 14, 2023 at 2:35 PM Javier Martinez Canillas
<javierm@xxxxxxxxxx> wrote:
> Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> writes:
> > On Fri, Jul 14, 2023 at 12:14 PM Javier Martinez Canillas
> > <javierm@xxxxxxxxxx> wrote:
> >> Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> writes:
> >> 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>

> >> > 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().
> >
> > The display should not be updated after .remove(), so I think plain
> > devm_kcalloc() should be fine.
>
> That was precisely my point, that there could be atomic commits even after
> the driver has been removed (e.g: if using DRM fbdev emulation, user-space
> can keep the /dev/fb0 opened and continue updating the framebuffer. That's
> not released until the fd is closed and struct fb_ops .fb_destroy called.
>
> But that's a general rule in DRM, any user-visible resource must not be
> allocated using device managed resources and instead use the drm_device
> managed resources helpers. To make sure that are not released until the
> last call to drm_dev_put():

These buffers are not user-visible, so they should not be accessed
after .remove().  When these are accessed, the next step would be
to write the buffer data to the device, which would also fail miserably,
as the regmap, GPIO, and regulator are hardware resources managed
through devm_*().

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds