Hi, On Thu, Jan 16, 2025 at 07:52:30PM +0100, Sasha Finkelstein wrote: > On Wed, 15 Jan 2025 at 11:21, Dmitry Baryshkov > <dmitry.baryshkov@xxxxxxxxxx> wrote: > > > + ret = drm_simple_encoder_init(drm, &adp->encoder, DRM_MODE_ENCODER_DSI); > > > > This is being deprecated, please use drm_encoder_init() / > > drmm_encoder_init() instead. > > I seem to be unable to find any documentation or a mailing list post > to this effect. If you are referring to this patchset: > https://patchwork.kernel.org/project/linux-samsung-soc/patch/20200313201744.19773-3-sam@xxxxxxxxxxxx/ > according to my understanding, it has not been applied. > In this case, the simple encoder api provides all functionality i need, > and a replacement would just be a simple encoder, except copied and pasted > into this driver. a drmm_encoder_(alloc|init)'d encoder with a NULL funcs pointer is strictly equivalent and doesn't require any extra infrastructure or work on your side compared to what you have here. > > > +static void adp_remove(struct platform_device *pdev) > > > +{ > > > + struct device *dev = &pdev->dev; > > > + struct drm_device *drm = dev_get_drvdata(dev); > > > + struct adp_drv_private *adp = to_adp(drm); > > > + > > > + adp_disable_vblank(adp); > > > > Isn't it being helped by the DRM itself? Anyway, I'd suggest moving it > > after DRM device unregistration and shutdown. > > Not sure i follow, as this call disables generation of vblank interrupts, > shouldn't it be done before all drm structures are torn down, to prevent > the interrupt handler from potentially operating on objects that are in > an invalid state? Your adp_crtc_atomic_disable() function calls drm_crtc_vblank_off() already. drm_atomic_helper_shutdown() will call the CRTC disable hook if relevant, so you're effectively disabling vblank twice here. Maxime
Attachment:
signature.asc
Description: PGP signature
