Re: [PATCH RFC v6 2/6] drm/sprd: add Unisoc's drm kms master

[Sam Ravnborg <sam@ravnborg.org>]

Hi Kevin

> >
> > > +
> > > +     drm->dev_private = sprd;
> > dev_private is deprecated. Alwyas use upclassing.
> dev_private is deprecated ? I see everyone is still using it, so it
> will be deprecated in the future?
It is deprecated - and a lot of drivers is in need up an update to get
away from using it.

From the kernel-doc:

        /**
         * @dev_private:
         *
         * DRM driver private data. This is deprecated and should be left set to
         * NULL.
         *
         * Instead of using this pointer it is recommended that drivers use
         * drm_dev_init() and embed struct &drm_device in their larger
         * per-device structure.
         */
        void *dev_private;

> > > +     }
> > > +     /* with irq_enabled = true, we can use the vblank feature. */
> > > +     drm->irq_enabled = true;
> > Can drm_irq_install() be used?
> > Then this flag shall not be set by the driver, And the interrupt numbers on different Soc are not necessarily the same
> >
> We need to set "IRQ_NOAUTOEN" flag for CRTC IRQ, you can see that we
> manually turn the interrupt on and off by crtc
> atomic_enable/atomic_disable
> and the interrupt number on different Soc are not necessarily the
> same. So intall interrupt in kms maybe is not suitable for us...

Maybe drm_irq_install() does not fit the need here. But please take an
extra look as what is does is quite basic. And no need to roll your
own if there is available infrstructure that can be used.

But there is nothing wrong rolloing your own if needed.

	Sam