On Mon, Sep 09, 2019 at 04:06:33PM +0200, Thomas Zimmermann wrote:
> Support for vblank requires VSYNC to signal an interrupt, which is broken
> on Matrox chipsets. The workaround that is used here and in other free
> Matrox drivers is to program <linecomp> to the value of <vdisplay> and
> enable the VLINE interrupt. This triggers an interrupt at the same time
> when VSYNC begins.
>
> VLINE uses separate registers for enabling and clearing pending interrupts.
> No extra syncronization between irq handler and the rest of the driver is
> required.
Looks good overall, some minor nits ...
> +irqreturn_t mgag200_irq_handler(int irq, void *arg)
> +{
> + struct drm_device *dev = arg;
> + struct mga_device *mdev = dev->dev_private;
> + struct drm_crtc *crtc;
> + u32 status, iclear;
> +
> + status = RREG32(0x1e14);
> +
> + if (status & 0x00000020) { /* test <vlinepen> */
> + drm_for_each_crtc(crtc, dev) {
> + drm_crtc_handle_vblank(crtc);
> + }
> + iclear = RREG32(0x1e18);
> + iclear |= 0x00000020; /* set <vlineiclr> */
#define for this would be good (you also don't need the comment then).
> + /* The VSYNC interrupt is broken on Matrox chipsets. We use
Codestyle. "/*" should be on a separate line.
> +static void mga_crtc_disable_vblank(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct mga_device *mdev = dev->dev_private;
> + u32 ien;
> +
> + ien = RREG32(0x1e1c);
> + ien &= 0xffffffdf; /* clear <vlineien> */
That is typically written as value &= ~(bit);
cheers,
Gerd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
