[PATCH v2 1/4] drm/via: drop use of DRM(READ|WRITE) macros

Emil Velikov emil.l.velikov at gmail.com
Mon Jul 22 16:27:28 UTC 2019


On Mon, 22 Jul 2019 at 17:17, Sam Ravnborg <sam at ravnborg.org> wrote:
>
> Hi Emil.
>
> On Mon, Jul 22, 2019 at 04:38:39PM +0100, Emil Velikov wrote:
> > On Sat, 20 Jul 2019 at 09:46, Sam Ravnborg <sam at ravnborg.org> wrote:
> > >
> > > The DRM_READ, DRM_WRITE macros comes from the deprecated drm_os_linux.h
> > > header file. Remove their use to remove this dependency.
> > >
> > > Replace the use of the macros with open coded variants.
> > >
> > > Signed-off-by: Sam Ravnborg <sam at ravnborg.org>
> > > Cc: Kevin Brace <kevinbrace at gmx.com>
> > > Cc: Thomas Hellstrom <thellstrom at vmware.com>
> > > Cc: "Gustavo A. R. Silva" <gustavo at embeddedor.com>
> > > Cc: Mike Marshall <hubcap at omnibond.com>
> > > Cc: Ira Weiny <ira.weiny at intel.com>
> > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > Cc: Emil Velikov <emil.velikov at collabora.com>
> > > Cc: Michel Dänzer <michel at daenzer.net>
> > > ---
> > >  drivers/gpu/drm/via/via_drv.h | 15 +++++++++++----
> > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/via/via_drv.h b/drivers/gpu/drm/via/via_drv.h
> > > index 6d1ae834484c..d5a2b1ffd8c1 100644
> > > --- a/drivers/gpu/drm/via/via_drv.h
> > > +++ b/drivers/gpu/drm/via/via_drv.h
> > > @@ -115,10 +115,17 @@ enum via_family {
> > >  /* VIA MMIO register access */
> > >  #define VIA_BASE ((dev_priv->mmio))
> > >
> > > -#define VIA_READ(reg)          DRM_READ32(VIA_BASE, reg)
> > > -#define VIA_WRITE(reg, val)    DRM_WRITE32(VIA_BASE, reg, val)
> > > -#define VIA_READ8(reg)         DRM_READ8(VIA_BASE, reg)
> > > -#define VIA_WRITE8(reg, val)   DRM_WRITE8(VIA_BASE, reg, val)
> > > +#define VIA_READ(reg) \
> > > +       readl(((void __iomem *)VIA_BASE->handle) + (reg))
> > > +
> > > +#define VIA_WRITE(reg, val) \
> > > +       writel(val, ((void __iomem *)VIA_BASE->handle) + (reg))
> > > +
> > > +#define VIA_READ8(reg) \
> > > +       readb(((void __iomem *)VIA_BASE->handle) + (reg))
> > > +
> > > +#define VIA_WRITE8(reg, val) \
> > > +       writeb(val, ((void __iomem *)VIA_BASE->handle) + (reg))
> > >
> > IMHO a far better idea is to expand these macros as static inline functions.
> > The extra bonus here is that the pseudo-magical VIA_BASE will also disappear.
> >
> > Since all the VIA_READ8 are used for masking, one might as well drop
> > them in favour of via_mask().
> >
> > WDYT?
>
> As this is a legacy driver I like the steps to be small.
> So we keep it like this but maybe address it in a follow-up patch?
>
Sounds like unnecessary churn BTH.

Looking from another angle - machines with such GPUs are likely to be
slow at building the kernel.
So people testing this, then another series (or two?) which does the
above polish would be time consuming.
Perhaps even a bit annoying.

That said, I don't have a strong preference.

-Emil


More information about the dri-devel mailing list