| Message ID | 20190608081923.6274-2-sam@ravnborg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/{mga,mgag200}: drop use of drmP.h | expand |
Hi Am 08.06.19 um 10:19 schrieb Sam Ravnborg: > Opencode all macros used from the deprecated drm_os_linux.h header file. > The DRM_WAIT_ON used 3 * HZ as timeout. > This was translated to 30 msec. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: David Airlie <airlied@linux.ie> > --- > drivers/gpu/drm/mga/mga_dma.c | 12 ++++++++---- > drivers/gpu/drm/mga/mga_drv.h | 12 ++++++++---- > drivers/gpu/drm/mga/mga_irq.c | 8 ++++---- > drivers/gpu/drm/mga/mga_state.c | 6 +++--- > 4 files changed, 23 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/mga/mga_dma.c b/drivers/gpu/drm/mga/mga_dma.c > index 1ffdafea27e4..123be2f3a151 100644 > --- a/drivers/gpu/drm/mga/mga_dma.c > +++ b/drivers/gpu/drm/mga/mga_dma.c > @@ -35,6 +35,8 @@ > * \author Gareth Hughes <gareth@valinux.com> > */ > > +#include <linux/delay.h> > + > #include <drm/drmP.h> > #include <drm/mga_drm.h> > #include "mga_drv.h" > @@ -62,7 +64,7 @@ int mga_do_wait_for_idle(drm_mga_private_t *dev_priv) > MGA_WRITE8(MGA_CRTC_INDEX, 0); > return 0; > } > - DRM_UDELAY(1); > + udelay(1); > } > > #if MGA_DMA_DEBUG > @@ -114,7 +116,7 @@ void mga_do_dma_flush(drm_mga_private_t *dev_priv) > status = MGA_READ(MGA_STATUS) & MGA_ENGINE_IDLE_MASK; > if (status == MGA_ENDPRDMASTS) > break; > - DRM_UDELAY(1); > + udelay(1); > } > > if (primary->tail == primary->last_flush) { > @@ -1120,7 +1122,7 @@ int mga_dma_buffers(struct drm_device *dev, void *data, > */ > if (d->send_count != 0) { > DRM_ERROR("Process %d trying to send %d buffers via drmDMA\n", > - DRM_CURRENTPID, d->send_count); > + task_pid_nr(current), d->send_count); > return -EINVAL; > } > > @@ -1128,7 +1130,9 @@ int mga_dma_buffers(struct drm_device *dev, void *data, > */ > if (d->request_count < 0 || d->request_count > dma->buf_count) { > DRM_ERROR("Process %d trying to get %d buffers (of %d max)\n", > - DRM_CURRENTPID, d->request_count, dma->buf_count); > + task_pid_nr(current), > + d->request_count, > + dma->buf_count); > return -EINVAL; > } > > diff --git a/drivers/gpu/drm/mga/mga_drv.h b/drivers/gpu/drm/mga/mga_drv.h > index a45bb22275a7..7844a9e463f6 100644 > --- a/drivers/gpu/drm/mga/mga_drv.h > +++ b/drivers/gpu/drm/mga/mga_drv.h > @@ -199,10 +199,14 @@ extern long mga_compat_ioctl(struct file *filp, unsigned int cmd, > > #define mga_flush_write_combine() wmb() > > -#define MGA_READ8(reg) DRM_READ8(dev_priv->mmio, (reg)) > -#define MGA_READ(reg) DRM_READ32(dev_priv->mmio, (reg)) > -#define MGA_WRITE8(reg, val) DRM_WRITE8(dev_priv->mmio, (reg), (val)) > -#define MGA_WRITE(reg, val) DRM_WRITE32(dev_priv->mmio, (reg), (val)) > +#define MGA_READ8(reg) \ > + readb(((void __iomem *)dev_priv->mmio->handle) + (reg)) > +#define MGA_READ(reg) \ > + readl(((void __iomem *)dev_priv->mmio->handle) + (reg)) > +#define MGA_WRITE8(reg, val) \ > + writeb(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) > +#define MGA_WRITE(reg, val) \ > + writel(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) Addition is not defined or implementation specific for type void* IIRC. Compilers tend to treat it like u8*. Maybe cast mmio->handle to (u8 __iomem *) instead? Best regards Thomas > > #define DWGREG0 0x1c00 > #define DWGREG0_END 0x1dff > diff --git a/drivers/gpu/drm/mga/mga_irq.c b/drivers/gpu/drm/mga/mga_irq.c > index 693ba708cfed..c6a3fab5b0c4 100644 > --- a/drivers/gpu/drm/mga/mga_irq.c > +++ b/drivers/gpu/drm/mga/mga_irq.c > @@ -122,19 +122,19 @@ int mga_driver_fence_wait(struct drm_device *dev, unsigned int *sequence) > { > drm_mga_private_t *dev_priv = (drm_mga_private_t *) dev->dev_private; > unsigned int cur_fence; > - int ret = 0; > > /* Assume that the user has missed the current sequence number > * by about a day rather than she wants to wait for years > * using fences. > */ > - DRM_WAIT_ON(ret, dev_priv->fence_queue, 3 * HZ, > + wait_event_timeout(dev_priv->fence_queue, > (((cur_fence = atomic_read(&dev_priv->last_fence_retired)) > - - *sequence) <= (1 << 23))); > + - *sequence) <= (1 << 23)), > + msecs_to_jiffies(30)); > > *sequence = cur_fence; > > - return ret; > + return 0; > } > > void mga_driver_irq_preinstall(struct drm_device *dev) > diff --git a/drivers/gpu/drm/mga/mga_state.c b/drivers/gpu/drm/mga/mga_state.c > index e5f6b735f575..296a1db7e5ee 100644 > --- a/drivers/gpu/drm/mga/mga_state.c > +++ b/drivers/gpu/drm/mga/mga_state.c > @@ -1016,7 +1016,7 @@ int mga_getparam(struct drm_device *dev, void *data, struct drm_file *file_priv) > return -EINVAL; > } > > - DRM_DEBUG("pid=%d\n", DRM_CURRENTPID); > + DRM_DEBUG("pid=%d\n", task_pid_nr(current)); > > switch (param->param) { > case MGA_PARAM_IRQ_NR: > @@ -1048,7 +1048,7 @@ static int mga_set_fence(struct drm_device *dev, void *data, struct drm_file *fi > return -EINVAL; > } > > - DRM_DEBUG("pid=%d\n", DRM_CURRENTPID); > + DRM_DEBUG("pid=%d\n", task_pid_nr(current)); > > /* I would normal do this assignment in the declaration of fence, > * but dev_priv may be NULL. > @@ -1077,7 +1077,7 @@ file_priv) > return -EINVAL; > } > > - DRM_DEBUG("pid=%d\n", DRM_CURRENTPID); > + DRM_DEBUG("pid=%d\n", task_pid_nr(current)); > > mga_driver_fence_wait(dev, fence); > return 0; >
On Tue, Jun 11, 2019 at 09:48:16AM +0200, Thomas Zimmermann wrote: > Hi > > Am 08.06.19 um 10:19 schrieb Sam Ravnborg: > > Opencode all macros used from the deprecated drm_os_linux.h header file. > > The DRM_WAIT_ON used 3 * HZ as timeout. > > This was translated to 30 msec. > > > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > Cc: David Airlie <airlied@linux.ie> > > --- > > drivers/gpu/drm/mga/mga_dma.c | 12 ++++++++---- > > drivers/gpu/drm/mga/mga_drv.h | 12 ++++++++---- > > drivers/gpu/drm/mga/mga_irq.c | 8 ++++---- > > drivers/gpu/drm/mga/mga_state.c | 6 +++--- > > 4 files changed, 23 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/mga/mga_dma.c b/drivers/gpu/drm/mga/mga_dma.c > > index 1ffdafea27e4..123be2f3a151 100644 > > --- a/drivers/gpu/drm/mga/mga_dma.c > > +++ b/drivers/gpu/drm/mga/mga_dma.c > > @@ -35,6 +35,8 @@ > > * \author Gareth Hughes <gareth@valinux.com> > > */ > > > > +#include <linux/delay.h> > > + > > #include <drm/drmP.h> > > #include <drm/mga_drm.h> > > #include "mga_drv.h" > > @@ -62,7 +64,7 @@ int mga_do_wait_for_idle(drm_mga_private_t *dev_priv) > > MGA_WRITE8(MGA_CRTC_INDEX, 0); > > return 0; > > } > > - DRM_UDELAY(1); > > + udelay(1); > > } > > > > #if MGA_DMA_DEBUG > > @@ -114,7 +116,7 @@ void mga_do_dma_flush(drm_mga_private_t *dev_priv) > > status = MGA_READ(MGA_STATUS) & MGA_ENGINE_IDLE_MASK; > > if (status == MGA_ENDPRDMASTS) > > break; > > - DRM_UDELAY(1); > > + udelay(1); > > } > > > > if (primary->tail == primary->last_flush) { > > @@ -1120,7 +1122,7 @@ int mga_dma_buffers(struct drm_device *dev, void *data, > > */ > > if (d->send_count != 0) { > > DRM_ERROR("Process %d trying to send %d buffers via drmDMA\n", > > - DRM_CURRENTPID, d->send_count); > > + task_pid_nr(current), d->send_count); > > return -EINVAL; > > } > > > > @@ -1128,7 +1130,9 @@ int mga_dma_buffers(struct drm_device *dev, void *data, > > */ > > if (d->request_count < 0 || d->request_count > dma->buf_count) { > > DRM_ERROR("Process %d trying to get %d buffers (of %d max)\n", > > - DRM_CURRENTPID, d->request_count, dma->buf_count); > > + task_pid_nr(current), > > + d->request_count, > > + dma->buf_count); > > return -EINVAL; > > } > > > > diff --git a/drivers/gpu/drm/mga/mga_drv.h b/drivers/gpu/drm/mga/mga_drv.h > > index a45bb22275a7..7844a9e463f6 100644 > > --- a/drivers/gpu/drm/mga/mga_drv.h > > +++ b/drivers/gpu/drm/mga/mga_drv.h > > @@ -199,10 +199,14 @@ extern long mga_compat_ioctl(struct file *filp, unsigned int cmd, > > > > #define mga_flush_write_combine() wmb() > > > > -#define MGA_READ8(reg) DRM_READ8(dev_priv->mmio, (reg)) > > -#define MGA_READ(reg) DRM_READ32(dev_priv->mmio, (reg)) > > -#define MGA_WRITE8(reg, val) DRM_WRITE8(dev_priv->mmio, (reg), (val)) > > -#define MGA_WRITE(reg, val) DRM_WRITE32(dev_priv->mmio, (reg), (val)) > > +#define MGA_READ8(reg) \ > > + readb(((void __iomem *)dev_priv->mmio->handle) + (reg)) > > +#define MGA_READ(reg) \ > > + readl(((void __iomem *)dev_priv->mmio->handle) + (reg)) > > +#define MGA_WRITE8(reg, val) \ > > + writeb(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) > > +#define MGA_WRITE(reg, val) \ > > + writel(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) > > Addition is not defined or implementation specific for type void* IIRC. > Compilers tend to treat it like u8*. Maybe cast mmio->handle to (u8 > __iomem *) instead? drm_os_linux.h has depended upon this gcc-ism since decades by now, I think it's ok to leave this as-is :-) -Daniel > > Best regards > Thomas > > > > > #define DWGREG0 0x1c00 > > #define DWGREG0_END 0x1dff > > diff --git a/drivers/gpu/drm/mga/mga_irq.c b/drivers/gpu/drm/mga/mga_irq.c > > index 693ba708cfed..c6a3fab5b0c4 100644 > > --- a/drivers/gpu/drm/mga/mga_irq.c > > +++ b/drivers/gpu/drm/mga/mga_irq.c > > @@ -122,19 +122,19 @@ int mga_driver_fence_wait(struct drm_device *dev, unsigned int *sequence) > > { > > drm_mga_private_t *dev_priv = (drm_mga_private_t *) dev->dev_private; > > unsigned int cur_fence; > > - int ret = 0; > > > > /* Assume that the user has missed the current sequence number > > * by about a day rather than she wants to wait for years > > * using fences. > > */ > > - DRM_WAIT_ON(ret, dev_priv->fence_queue, 3 * HZ, > > + wait_event_timeout(dev_priv->fence_queue, > > (((cur_fence = atomic_read(&dev_priv->last_fence_retired)) > > - - *sequence) <= (1 << 23))); > > + - *sequence) <= (1 << 23)), > > + msecs_to_jiffies(30)); > > > > *sequence = cur_fence; > > > > - return ret; > > + return 0; > > } > > > > void mga_driver_irq_preinstall(struct drm_device *dev) > > diff --git a/drivers/gpu/drm/mga/mga_state.c b/drivers/gpu/drm/mga/mga_state.c > > index e5f6b735f575..296a1db7e5ee 100644 > > --- a/drivers/gpu/drm/mga/mga_state.c > > +++ b/drivers/gpu/drm/mga/mga_state.c > > @@ -1016,7 +1016,7 @@ int mga_getparam(struct drm_device *dev, void *data, struct drm_file *file_priv) > > return -EINVAL; > > } > > > > - DRM_DEBUG("pid=%d\n", DRM_CURRENTPID); > > + DRM_DEBUG("pid=%d\n", task_pid_nr(current)); > > > > switch (param->param) { > > case MGA_PARAM_IRQ_NR: > > @@ -1048,7 +1048,7 @@ static int mga_set_fence(struct drm_device *dev, void *data, struct drm_file *fi > > return -EINVAL; > > } > > > > - DRM_DEBUG("pid=%d\n", DRM_CURRENTPID); > > + DRM_DEBUG("pid=%d\n", task_pid_nr(current)); > > > > /* I would normal do this assignment in the declaration of fence, > > * but dev_priv may be NULL. > > @@ -1077,7 +1077,7 @@ file_priv) > > return -EINVAL; > > } > > > > - DRM_DEBUG("pid=%d\n", DRM_CURRENTPID); > > + DRM_DEBUG("pid=%d\n", task_pid_nr(current)); > > > > mga_driver_fence_wait(dev, fence); > > return 0; > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah > HRB 21284 (AG Nürnberg) >
Hi Thomas. > > +#define MGA_READ8(reg) \ > > + readb(((void __iomem *)dev_priv->mmio->handle) + (reg)) > > +#define MGA_READ(reg) \ > > + readl(((void __iomem *)dev_priv->mmio->handle) + (reg)) > > +#define MGA_WRITE8(reg, val) \ > > + writeb(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) > > +#define MGA_WRITE(reg, val) \ > > + writel(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) > > Addition is not defined or implementation specific for type void* IIRC. > Compilers tend to treat it like u8*. Maybe cast mmio->handle to (u8 > __iomem *) instead? I briefly looked at changing the type of mmio->handle Today: void *handle; /**< User-space: "Handle" to pass to mmap() */ Proposal: void __iomem *handle; /**< User-space: "Handle" to pass to mmap() */ This would allow me to drop the cast in the code above, that is only added to make sparse happy. But the above triggered other sparse warnings and I ended up dropping this. As for (void *) versus (u8 *), then (void *) is what we do today. [What Daniel also says in another mail]. Should we change the type I would prepfer a follow-up patch to do it. If you could test it I can type the patch, or you could do so when working with the driver. Sam
Hi Am 11.06.19 um 12:44 schrieb Sam Ravnborg: > Hi Thomas. > >>> +#define MGA_READ8(reg) \ >>> + readb(((void __iomem *)dev_priv->mmio->handle) + (reg)) >>> +#define MGA_READ(reg) \ >>> + readl(((void __iomem *)dev_priv->mmio->handle) + (reg)) >>> +#define MGA_WRITE8(reg, val) \ >>> + writeb(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) >>> +#define MGA_WRITE(reg, val) \ >>> + writel(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) >> >> Addition is not defined or implementation specific for type void* IIRC. >> Compilers tend to treat it like u8*. Maybe cast mmio->handle to (u8 >> __iomem *) instead? > I briefly looked at changing the type of mmio->handle > > Today: > > void *handle; /**< User-space: "Handle" to pass to mmap() */ > > Proposal: > void __iomem *handle; /**< User-space: "Handle" to pass to mmap() */ > > This would allow me to drop the cast in the code above, that is only > added to make sparse happy. > But the above triggered other sparse warnings and I ended up dropping > this. > > As for (void *) versus (u8 *), then (void *) is what we do today. > [What Daniel also says in another mail]. > > Should we change the type I would prepfer a follow-up patch to do it. > > If you could test it I can type the patch, or you could do so when > working with the driver. I don't really do anything with mga, so my testing is as good as anyone's. But it's not that important. Don't make this patch set depend on such mostly cosmetic issues. Best regards Thomas > > Sam > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
diff --git a/drivers/gpu/drm/mga/mga_dma.c b/drivers/gpu/drm/mga/mga_dma.c index 1ffdafea27e4..123be2f3a151 100644 --- a/drivers/gpu/drm/mga/mga_dma.c +++ b/drivers/gpu/drm/mga/mga_dma.c @@ -35,6 +35,8 @@ * \author Gareth Hughes <gareth@valinux.com> */ +#include <linux/delay.h> + #include <drm/drmP.h> #include <drm/mga_drm.h> #include "mga_drv.h" @@ -62,7 +64,7 @@ int mga_do_wait_for_idle(drm_mga_private_t *dev_priv) MGA_WRITE8(MGA_CRTC_INDEX, 0); return 0; } - DRM_UDELAY(1); + udelay(1); } #if MGA_DMA_DEBUG @@ -114,7 +116,7 @@ void mga_do_dma_flush(drm_mga_private_t *dev_priv) status = MGA_READ(MGA_STATUS) & MGA_ENGINE_IDLE_MASK; if (status == MGA_ENDPRDMASTS) break; - DRM_UDELAY(1); + udelay(1); } if (primary->tail == primary->last_flush) { @@ -1120,7 +1122,7 @@ int mga_dma_buffers(struct drm_device *dev, void *data, */ if (d->send_count != 0) { DRM_ERROR("Process %d trying to send %d buffers via drmDMA\n", - DRM_CURRENTPID, d->send_count); + task_pid_nr(current), d->send_count); return -EINVAL; } @@ -1128,7 +1130,9 @@ int mga_dma_buffers(struct drm_device *dev, void *data, */ if (d->request_count < 0 || d->request_count > dma->buf_count) { DRM_ERROR("Process %d trying to get %d buffers (of %d max)\n", - DRM_CURRENTPID, d->request_count, dma->buf_count); + task_pid_nr(current), + d->request_count, + dma->buf_count); return -EINVAL; } diff --git a/drivers/gpu/drm/mga/mga_drv.h b/drivers/gpu/drm/mga/mga_drv.h index a45bb22275a7..7844a9e463f6 100644 --- a/drivers/gpu/drm/mga/mga_drv.h +++ b/drivers/gpu/drm/mga/mga_drv.h @@ -199,10 +199,14 @@ extern long mga_compat_ioctl(struct file *filp, unsigned int cmd, #define mga_flush_write_combine() wmb() -#define MGA_READ8(reg) DRM_READ8(dev_priv->mmio, (reg)) -#define MGA_READ(reg) DRM_READ32(dev_priv->mmio, (reg)) -#define MGA_WRITE8(reg, val) DRM_WRITE8(dev_priv->mmio, (reg), (val)) -#define MGA_WRITE(reg, val) DRM_WRITE32(dev_priv->mmio, (reg), (val)) +#define MGA_READ8(reg) \ + readb(((void __iomem *)dev_priv->mmio->handle) + (reg)) +#define MGA_READ(reg) \ + readl(((void __iomem *)dev_priv->mmio->handle) + (reg)) +#define MGA_WRITE8(reg, val) \ + writeb(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) +#define MGA_WRITE(reg, val) \ + writel(val, ((void __iomem *)dev_priv->mmio->handle) + (reg)) #define DWGREG0 0x1c00 #define DWGREG0_END 0x1dff diff --git a/drivers/gpu/drm/mga/mga_irq.c b/drivers/gpu/drm/mga/mga_irq.c index 693ba708cfed..c6a3fab5b0c4 100644 --- a/drivers/gpu/drm/mga/mga_irq.c +++ b/drivers/gpu/drm/mga/mga_irq.c @@ -122,19 +122,19 @@ int mga_driver_fence_wait(struct drm_device *dev, unsigned int *sequence) { drm_mga_private_t *dev_priv = (drm_mga_private_t *) dev->dev_private; unsigned int cur_fence; - int ret = 0; /* Assume that the user has missed the current sequence number * by about a day rather than she wants to wait for years * using fences. */ - DRM_WAIT_ON(ret, dev_priv->fence_queue, 3 * HZ, + wait_event_timeout(dev_priv->fence_queue, (((cur_fence = atomic_read(&dev_priv->last_fence_retired)) - - *sequence) <= (1 << 23))); + - *sequence) <= (1 << 23)), + msecs_to_jiffies(30)); *sequence = cur_fence; - return ret; + return 0; } void mga_driver_irq_preinstall(struct drm_device *dev) diff --git a/drivers/gpu/drm/mga/mga_state.c b/drivers/gpu/drm/mga/mga_state.c index e5f6b735f575..296a1db7e5ee 100644 --- a/drivers/gpu/drm/mga/mga_state.c +++ b/drivers/gpu/drm/mga/mga_state.c @@ -1016,7 +1016,7 @@ int mga_getparam(struct drm_device *dev, void *data, struct drm_file *file_priv) return -EINVAL; } - DRM_DEBUG("pid=%d\n", DRM_CURRENTPID); + DRM_DEBUG("pid=%d\n", task_pid_nr(current)); switch (param->param) { case MGA_PARAM_IRQ_NR: @@ -1048,7 +1048,7 @@ static int mga_set_fence(struct drm_device *dev, void *data, struct drm_file *fi return -EINVAL; } - DRM_DEBUG("pid=%d\n", DRM_CURRENTPID); + DRM_DEBUG("pid=%d\n", task_pid_nr(current)); /* I would normal do this assignment in the declaration of fence, * but dev_priv may be NULL. @@ -1077,7 +1077,7 @@ file_priv) return -EINVAL; } - DRM_DEBUG("pid=%d\n", DRM_CURRENTPID); + DRM_DEBUG("pid=%d\n", task_pid_nr(current)); mga_driver_fence_wait(dev, fence); return 0;
Opencode all macros used from the deprecated drm_os_linux.h header file. The DRM_WAIT_ON used 3 * HZ as timeout. This was translated to 30 msec. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@linux.ie> --- drivers/gpu/drm/mga/mga_dma.c | 12 ++++++++---- drivers/gpu/drm/mga/mga_drv.h | 12 ++++++++---- drivers/gpu/drm/mga/mga_irq.c | 8 ++++---- drivers/gpu/drm/mga/mga_state.c | 6 +++--- 4 files changed, 23 insertions(+), 15 deletions(-)