diff mbox series

[v2,3/3] drm/mgag200: Add vblank support

Message ID 20190912064230.27972-4-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series Rate-limit shadow-FB-to-console-update to screen refresh | expand

Commit Message

Thomas Zimmermann Sept. 12, 2019, 6:42 a.m. UTC 
There's no VBLANK interrupt on Matrox chipsets. The workaround that is
being 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 time when VBLANK 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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

v2:
	* only signal vblank on CRTC 0
	* use constants for registers and fields
	* set VLINECLR before enabling interrupt
	* test against STATUS and IEN in irq handler
	* coding-style fixes
---
 drivers/gpu/drm/mgag200/mgag200_drv.c  |  1 +
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  1 +
 drivers/gpu/drm/mgag200/mgag200_main.c | 40 ++++++++++++++++++++++
 drivers/gpu/drm/mgag200/mgag200_mode.c | 47 +++++++++++++++++++++++---
 drivers/gpu/drm/mgag200/mgag200_reg.h  |  5 +++
 5 files changed, 90 insertions(+), 4 deletions(-)

Comments

Gerd Hoffmann Sept. 12, 2019, 8:53 a.m. UTC | #1 
On Thu, Sep 12, 2019 at 08:42:30AM +0200, Thomas Zimmermann wrote:
> There's no VBLANK interrupt on Matrox chipsets. The workaround that is
> being 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 time when VBLANK 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.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Looks sane, can't justify on mga hardware details though.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

> 
> v2:
> 	* only signal vblank on CRTC 0
> 	* use constants for registers and fields
> 	* set VLINECLR before enabling interrupt
> 	* test against STATUS and IEN in irq handler
> 	* coding-style fixes
> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.c  |  1 +
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  1 +
>  drivers/gpu/drm/mgag200/mgag200_main.c | 40 ++++++++++++++++++++++
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 47 +++++++++++++++++++++++---
>  drivers/gpu/drm/mgag200/mgag200_reg.h  |  5 +++
>  5 files changed, 90 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index 4f9df3b93598..cff265973154 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -67,6 +67,7 @@ static struct drm_driver driver = {
>  	.driver_features = DRIVER_GEM | DRIVER_MODESET,
>  	.load = mgag200_driver_load,
>  	.unload = mgag200_driver_unload,
> +	.irq_handler = mgag200_irq_handler,
>  	.fops = &mgag200_driver_fops,
>  	.name = DRIVER_NAME,
>  	.desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 1c93f8dc08c7..88cf256d135f 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -195,6 +195,7 @@ void mgag200_modeset_fini(struct mga_device *mdev);
>  				/* mgag200_main.c */
>  int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
>  void mgag200_driver_unload(struct drm_device *dev);
> +irqreturn_t mgag200_irq_handler(int irq, void *arg);
>  
>  				/* mgag200_i2c.c */
>  struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
> index a9773334dedf..44273a66f5a5 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
> @@ -10,7 +10,9 @@
>  
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_irq.h>
>  #include <drm/drm_pci.h>
> +#include <drm/drm_vblank.h>
>  
>  #include "mgag200_drv.h"
>  
> @@ -186,10 +188,18 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
>  	}
>  	mdev->cursor.pixels_current = NULL;
>  
> +	r = drm_vblank_init(dev, 1);
> +	if (r)
> +		goto err_modeset;
> +
>  	r = drm_fbdev_generic_setup(mdev->dev, 0);
>  	if (r)
>  		goto err_modeset;
>  
> +	r = drm_irq_install(dev, dev->pdev->irq);
> +	if (r)
> +		goto err_modeset;
> +
>  	return 0;
>  
>  err_modeset:
> @@ -207,8 +217,38 @@ void mgag200_driver_unload(struct drm_device *dev)
>  
>  	if (mdev == NULL)
>  		return;
> +	drm_irq_uninstall(dev);
>  	mgag200_modeset_fini(mdev);
>  	drm_mode_config_cleanup(dev);
>  	mgag200_mm_fini(mdev);
>  	dev->dev_private = NULL;
>  }
> +
> +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, ien, iclear;
> +
> +	status = RREG32(MGAREG_STATUS);
> +
> +	if (status & MGA_VLINEPEN) {
> +		ien = RREG32(MGAREG_IEN);
> +		if (!(ien & MGA_VLINEIEN))
> +			goto out;
> +
> +		crtc = drm_crtc_from_index(dev, 0);
> +		if (WARN_ON_ONCE(!crtc))
> +			goto out;
> +		drm_crtc_handle_vblank(crtc);
> +
> +		iclear = RREG32(MGAREG_ICLEAR);
> +		iclear |= MGA_VLINECLR;
> +		WREG32(MGAREG_ICLEAR, iclear);
> +		return IRQ_HANDLED;
> +	}
> +
> +out:
> +	return IRQ_NONE;
> +};
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 302ba40eb033..e13c3244fea9 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -905,6 +905,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  	const struct drm_framebuffer *fb = crtc->primary->fb;
>  	int hdisplay, hsyncstart, hsyncend, htotal;
>  	int vdisplay, vsyncstart, vsyncend, vtotal;
> +	int linecomp;
>  	int pitch;
>  	int option = 0, option2 = 0;
>  	int i;
> @@ -1042,6 +1043,14 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  	vsyncend = mode->vsync_end - 1;
>  	vtotal = mode->vtotal - 2;
>  
> +	/*
> +	 * There's no VBLANK interrupt on Matrox chipsets, so we have to
> +	 * use the VLINE interrupt instead. It triggers when the current
> +	 * linecomp has been reached. Therefore keep <linecomp> in
> +	 * sync with <vdisplay>.
> +	 */
> +	linecomp = vdisplay;
> +
>  	WREG_GFX(0, 0);
>  	WREG_GFX(1, 0);
>  	WREG_GFX(2, 0);
> @@ -1063,12 +1072,12 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  		 ((vdisplay & 0x100) >> 7) |
>  		 ((vsyncstart & 0x100) >> 6) |
>  		 ((vdisplay & 0x100) >> 5) |
> -		 ((vdisplay & 0x100) >> 4) | /* linecomp */
> +		 ((linecomp & 0x100) >> 4) |
>  		 ((vtotal & 0x200) >> 4)|
>  		 ((vdisplay & 0x200) >> 3) |
>  		 ((vsyncstart & 0x200) >> 2));
>  	WREG_CRT(9, ((vdisplay & 0x200) >> 4) |
> -		 ((vdisplay & 0x200) >> 3));
> +		 ((linecomp & 0x200) >> 3));
>  	WREG_CRT(10, 0);
>  	WREG_CRT(11, 0);
>  	WREG_CRT(12, 0);
> @@ -1083,7 +1092,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  	WREG_CRT(21, vdisplay & 0xFF);
>  	WREG_CRT(22, (vtotal + 1) & 0xFF);
>  	WREG_CRT(23, 0xc3);
> -	WREG_CRT(24, vdisplay & 0xFF);
> +	WREG_CRT(24, linecomp & 0xff);
>  
>  	ext_vga[0] = 0;
>  	ext_vga[5] = 0;
> @@ -1099,7 +1108,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>  		((vdisplay & 0x400) >> 8) |
>  		((vdisplay & 0xc00) >> 7) |
>  		((vsyncstart & 0xc00) >> 5) |
> -		((vdisplay & 0x400) >> 3);
> +		((linecomp & 0x400) >> 3);
>  	if (fb->format->cpp[0] * 8 == 24)
>  		ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80;
>  	else
> @@ -1411,6 +1420,34 @@ static void mga_crtc_disable(struct drm_crtc *crtc)
>  	crtc->primary->fb = NULL;
>  }
>  
> +static int mga_crtc_enable_vblank(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct mga_device *mdev = dev->dev_private;
> +	u32 iclear, ien;
> +
> +	iclear = RREG32(MGAREG_ICLEAR);
> +	iclear |= MGA_VLINECLR;
> +	WREG32(MGAREG_ICLEAR, iclear);
> +
> +	ien = RREG32(MGAREG_IEN);
> +	ien |= MGA_VLINEIEN;
> +	WREG32(MGAREG_IEN, ien);
> +
> +	return 0;
> +}
> +
> +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(MGAREG_IEN);
> +	ien &= ~(MGA_VLINEIEN);
> +	WREG32(MGAREG_IEN, ien);
> +}
> +
>  /* These provide the minimum set of functions required to handle a CRTC */
>  static const struct drm_crtc_funcs mga_crtc_funcs = {
>  	.cursor_set = mga_crtc_cursor_set,
> @@ -1418,6 +1455,8 @@ static const struct drm_crtc_funcs mga_crtc_funcs = {
>  	.gamma_set = mga_crtc_gamma_set,
>  	.set_config = drm_crtc_helper_set_config,
>  	.destroy = mga_crtc_destroy,
> +	.enable_vblank = mga_crtc_enable_vblank,
> +	.disable_vblank = mga_crtc_disable_vblank,
>  };
>  
>  static const struct drm_crtc_helper_funcs mga_helper_funcs = {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h
> index 6c460d9a2143..44db1d8279fa 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_reg.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
> @@ -122,6 +122,11 @@
>  
>  #define MGAREG_MEMCTL           0x2e08
>  
> +/* Interrupt fields */
> +#define MGA_VLINEPEN		(0x01 << 5)
> +#define MGA_VLINECLR		(0x01 << 5)
> +#define MGA_VLINEIEN		(0x01 << 5)
> +
>  /* OPMODE register additives */
>  
>  #define MGAOPM_DMA_GENERAL	(0x00 << 2)
> -- 
> 2.23.0
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 4f9df3b93598..cff265973154 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -67,6 +67,7 @@  static struct drm_driver driver = {
 	.driver_features = DRIVER_GEM | DRIVER_MODESET,
 	.load = mgag200_driver_load,
 	.unload = mgag200_driver_unload,
+	.irq_handler = mgag200_irq_handler,
 	.fops = &mgag200_driver_fops,
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 1c93f8dc08c7..88cf256d135f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -195,6 +195,7 @@  void mgag200_modeset_fini(struct mga_device *mdev);
 				/* mgag200_main.c */
 int mgag200_driver_load(struct drm_device *dev, unsigned long flags);
 void mgag200_driver_unload(struct drm_device *dev);
+irqreturn_t mgag200_irq_handler(int irq, void *arg);
 
 				/* mgag200_i2c.c */
 struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index a9773334dedf..44273a66f5a5 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -10,7 +10,9 @@ 
 
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_irq.h>
 #include <drm/drm_pci.h>
+#include <drm/drm_vblank.h>
 
 #include "mgag200_drv.h"
 
@@ -186,10 +188,18 @@  int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 	}
 	mdev->cursor.pixels_current = NULL;
 
+	r = drm_vblank_init(dev, 1);
+	if (r)
+		goto err_modeset;
+
 	r = drm_fbdev_generic_setup(mdev->dev, 0);
 	if (r)
 		goto err_modeset;
 
+	r = drm_irq_install(dev, dev->pdev->irq);
+	if (r)
+		goto err_modeset;
+
 	return 0;
 
 err_modeset:
@@ -207,8 +217,38 @@  void mgag200_driver_unload(struct drm_device *dev)
 
 	if (mdev == NULL)
 		return;
+	drm_irq_uninstall(dev);
 	mgag200_modeset_fini(mdev);
 	drm_mode_config_cleanup(dev);
 	mgag200_mm_fini(mdev);
 	dev->dev_private = NULL;
 }
+
+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, ien, iclear;
+
+	status = RREG32(MGAREG_STATUS);
+
+	if (status & MGA_VLINEPEN) {
+		ien = RREG32(MGAREG_IEN);
+		if (!(ien & MGA_VLINEIEN))
+			goto out;
+
+		crtc = drm_crtc_from_index(dev, 0);
+		if (WARN_ON_ONCE(!crtc))
+			goto out;
+		drm_crtc_handle_vblank(crtc);
+
+		iclear = RREG32(MGAREG_ICLEAR);
+		iclear |= MGA_VLINECLR;
+		WREG32(MGAREG_ICLEAR, iclear);
+		return IRQ_HANDLED;
+	}
+
+out:
+	return IRQ_NONE;
+};
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 302ba40eb033..e13c3244fea9 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -905,6 +905,7 @@  static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	const struct drm_framebuffer *fb = crtc->primary->fb;
 	int hdisplay, hsyncstart, hsyncend, htotal;
 	int vdisplay, vsyncstart, vsyncend, vtotal;
+	int linecomp;
 	int pitch;
 	int option = 0, option2 = 0;
 	int i;
@@ -1042,6 +1043,14 @@  static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	vsyncend = mode->vsync_end - 1;
 	vtotal = mode->vtotal - 2;
 
+	/*
+	 * There's no VBLANK interrupt on Matrox chipsets, so we have to
+	 * use the VLINE interrupt instead. It triggers when the current
+	 * linecomp has been reached. Therefore keep <linecomp> in
+	 * sync with <vdisplay>.
+	 */
+	linecomp = vdisplay;
+
 	WREG_GFX(0, 0);
 	WREG_GFX(1, 0);
 	WREG_GFX(2, 0);
@@ -1063,12 +1072,12 @@  static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		 ((vdisplay & 0x100) >> 7) |
 		 ((vsyncstart & 0x100) >> 6) |
 		 ((vdisplay & 0x100) >> 5) |
-		 ((vdisplay & 0x100) >> 4) | /* linecomp */
+		 ((linecomp & 0x100) >> 4) |
 		 ((vtotal & 0x200) >> 4)|
 		 ((vdisplay & 0x200) >> 3) |
 		 ((vsyncstart & 0x200) >> 2));
 	WREG_CRT(9, ((vdisplay & 0x200) >> 4) |
-		 ((vdisplay & 0x200) >> 3));
+		 ((linecomp & 0x200) >> 3));
 	WREG_CRT(10, 0);
 	WREG_CRT(11, 0);
 	WREG_CRT(12, 0);
@@ -1083,7 +1092,7 @@  static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	WREG_CRT(21, vdisplay & 0xFF);
 	WREG_CRT(22, (vtotal + 1) & 0xFF);
 	WREG_CRT(23, 0xc3);
-	WREG_CRT(24, vdisplay & 0xFF);
+	WREG_CRT(24, linecomp & 0xff);
 
 	ext_vga[0] = 0;
 	ext_vga[5] = 0;
@@ -1099,7 +1108,7 @@  static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		((vdisplay & 0x400) >> 8) |
 		((vdisplay & 0xc00) >> 7) |
 		((vsyncstart & 0xc00) >> 5) |
-		((vdisplay & 0x400) >> 3);
+		((linecomp & 0x400) >> 3);
 	if (fb->format->cpp[0] * 8 == 24)
 		ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80;
 	else
@@ -1411,6 +1420,34 @@  static void mga_crtc_disable(struct drm_crtc *crtc)
 	crtc->primary->fb = NULL;
 }
 
+static int mga_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct mga_device *mdev = dev->dev_private;
+	u32 iclear, ien;
+
+	iclear = RREG32(MGAREG_ICLEAR);
+	iclear |= MGA_VLINECLR;
+	WREG32(MGAREG_ICLEAR, iclear);
+
+	ien = RREG32(MGAREG_IEN);
+	ien |= MGA_VLINEIEN;
+	WREG32(MGAREG_IEN, ien);
+
+	return 0;
+}
+
+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(MGAREG_IEN);
+	ien &= ~(MGA_VLINEIEN);
+	WREG32(MGAREG_IEN, ien);
+}
+
 /* These provide the minimum set of functions required to handle a CRTC */
 static const struct drm_crtc_funcs mga_crtc_funcs = {
 	.cursor_set = mga_crtc_cursor_set,
@@ -1418,6 +1455,8 @@  static const struct drm_crtc_funcs mga_crtc_funcs = {
 	.gamma_set = mga_crtc_gamma_set,
 	.set_config = drm_crtc_helper_set_config,
 	.destroy = mga_crtc_destroy,
+	.enable_vblank = mga_crtc_enable_vblank,
+	.disable_vblank = mga_crtc_disable_vblank,
 };
 
 static const struct drm_crtc_helper_funcs mga_helper_funcs = {
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h
index 6c460d9a2143..44db1d8279fa 100644
--- a/drivers/gpu/drm/mgag200/mgag200_reg.h
+++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
@@ -122,6 +122,11 @@ 
 
 #define MGAREG_MEMCTL           0x2e08
 
+/* Interrupt fields */
+#define MGA_VLINEPEN		(0x01 << 5)
+#define MGA_VLINECLR		(0x01 << 5)
+#define MGA_VLINEIEN		(0x01 << 5)
+
 /* OPMODE register additives */
 
 #define MGAOPM_DMA_GENERAL	(0x00 << 2)