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