On Mon, 22 Jul 2019 at 17:17, Sam Ravnborg <sam@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > Cc: Kevin Brace <kevinbrace@xxxxxxx>
> > > Cc: Thomas Hellstrom <thellstrom@xxxxxxxxxx>
> > > Cc: "Gustavo A. R. Silva" <gustavo@xxxxxxxxxxxxxx>
> > > Cc: Mike Marshall <hubcap@xxxxxxxxxxxx>
> > > Cc: Ira Weiny <ira.weiny@xxxxxxxxx>
> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > Cc: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
> > > Cc: Michel Dänzer <michel@xxxxxxxxxxx>
> > > ---
> > > 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
