[PATCH 1/3] drm/mgag200: Index 24 in extended CRTC registers is 24 in hex, not decimal.

[PATCH 1/3] drm/mgag200: Index 24 in extended CRTC registers is 24 in hex, not decimal.

[Christopher Harvey]

This change properly enables the "requester" in G200ER cards that is
responsible for getting pixels out of memory and clocking them out to
the screen.

Signed-off-by: Christopher Harvey <charvey@matrox.com>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 7337013..f988965 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -751,8 +751,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	int i;
 	unsigned char misc = 0;
 	unsigned char ext_vga[6];
-	unsigned char ext_vga_index24;
-	unsigned char dac_index90 = 0;
 	u8 bppshift;
 
 	static unsigned char dacvalue[] = {
@@ -803,7 +801,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		option2 = 0x0000b000;
 		break;
 	case G200_ER:
-		dac_index90 = 0;
 		break;
 	}
 
@@ -852,10 +849,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		WREG_DAC(i, dacvalue[i]);
 	}
 
-	if (mdev->type == G200_ER) {
-		WREG_DAC(0x90, dac_index90);
-	}
-
+	if (mdev->type == G200_ER)
+		WREG_DAC(0x90, 0);
 
 	if (option)
 		pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option);
@@ -952,8 +947,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	if (mdev->type == G200_WB)
 		ext_vga[1] |= 0x88;
 
-	ext_vga_index24 = 0x05;
-
 	/* Set pixel clocks */
 	misc = 0x2d;
 	WREG8(MGA_MISC_OUT, misc);
@@ -965,7 +958,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	}
 
 	if (mdev->type == G200_ER)
-		WREG_ECRT(24, ext_vga_index24);
+		WREG_ECRT(0x24, 0x5);
 
 	if (mdev->type == G200_EV) {
 		WREG_ECRT(6, 0);
-- 
1.7.12.4

[PATCH 2/3] drm: Misc comment cleanup

[Christopher Harvey]

Signed-off-by: Christopher Harvey <charvey@matrox.com>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h | 6 +++---
 include/drm/drm_crtc.h                | 2 +-
 include/uapi/drm/drm_mode.h           | 6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 4d932c4..dcfc973 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -215,7 +215,7 @@ mgag200_bo(struct ttm_buffer_object *bo)
 {
 	return container_of(bo, struct mgag200_bo, bo);
 }
-				/* mga_crtc.c */
+				/* mgag200_crtc.c */
 void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
 			     u16 blue, int regno);
 void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
@@ -225,7 +225,7 @@ void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
 int mgag200_modeset_init(struct mga_device *mdev);
 void mgag200_modeset_fini(struct mga_device *mdev);
 
-				/* mga_fbdev.c */
+				/* mgag200_fb.c */
 int mgag200_fbdev_init(struct mga_device *mdev);
 void mgag200_fbdev_fini(struct mga_device *mdev);
 
@@ -254,7 +254,7 @@ mgag200_dumb_mmap_offset(struct drm_file *file,
 			 struct drm_device *dev,
 			 uint32_t handle,
 			 uint64_t *offset);
-				/* mga_i2c.c */
+				/* mgag200_i2c.c */
 struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
 void mgag200_i2c_destroy(struct mga_i2c_chan *i2c);
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e3e0d65..8c7846b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -310,7 +310,7 @@ struct drm_plane;
  * drm_crtc_funcs - control CRTCs for a given device
  * @save: save CRTC state
  * @restore: restore CRTC state
- * @reset: reset CRTC after state has been invalidate (e.g. resume)
+ * @reset: reset CRTC after state has been invalidated (e.g. resume)
  * @cursor_set: setup the cursor
  * @cursor_move: move the cursor
  * @gamma_set: specify color ramp for CRTC
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 3d6301b..090e533 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -367,13 +367,13 @@ struct drm_mode_mode_cmd {
  * depending on the value in flags different members are used.
  *
  * CURSOR_BO uses
- *    crtc
+ *    crtc_id
  *    width
  *    height
- *    handle - if 0 turns the cursor of
+ *    handle - if 0 turns the cursor off
  *
  * CURSOR_MOVE uses
- *    crtc
+ *    crtc_id
  *    x
  *    y
  */
-- 
1.7.12.4

[PATCH 3/3] drm/mgag200: Convert to managed device resources where possible

[Christopher Harvey]

Signed-off-by: Christopher Harvey <charvey@matrox.com>
---
 drivers/gpu/drm/mgag200/mgag200_fb.c   |  9 +++------
 drivers/gpu/drm/mgag200/mgag200_main.c | 29 +++++------------------------
 2 files changed, 8 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index a5a1f34..421beab 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -246,7 +246,7 @@ int mgag200_fbdev_init(struct mga_device *mdev)
 	struct mga_fbdev *mfbdev;
 	int ret;
 
-	mfbdev = kzalloc(sizeof(struct mga_fbdev), GFP_KERNEL);
+	mfbdev = devm_kzalloc(mdev->dev->dev, sizeof(struct mga_fbdev), GFP_KERNEL);
 	if (!mfbdev)
 		return -ENOMEM;
 
@@ -255,10 +255,9 @@ int mgag200_fbdev_init(struct mga_device *mdev)
 
 	ret = drm_fb_helper_init(mdev->dev, &mfbdev->helper,
 				 mdev->num_crtc, MGAG200FB_CONN_LIMIT);
-	if (ret) {
-		kfree(mfbdev);
+	if (ret)
 		return ret;
-	}
+
 	drm_fb_helper_single_add_all_connectors(&mfbdev->helper);
 
 	/* disable all the possible outputs/crtcs before entering KMS mode */
@@ -275,6 +274,4 @@ void mgag200_fbdev_fini(struct mga_device *mdev)
 		return;
 
 	mga_fbdev_destroy(mdev->dev, mdev->mfbdev);
-	kfree(mdev->mfbdev);
-	mdev->mfbdev = NULL;
 }
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index baf54d9..19db16d 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -80,15 +80,6 @@ static const struct drm_mode_config_funcs mga_mode_funcs = {
 	.fb_create = mgag200_user_framebuffer_create,
 };
 
-/* Unmap the framebuffer from the core and release the memory */
-static void mga_vram_fini(struct mga_device *mdev)
-{
-	pci_iounmap(mdev->dev->pdev, mdev->rmmio);
-	mdev->rmmio = NULL;
-	if (mdev->mc.vram_base)
-		release_mem_region(mdev->mc.vram_base, mdev->mc.vram_window);
-}
-
 static int mga_probe_vram(struct mga_device *mdev, void __iomem *mem)
 {
 	int offset;
@@ -144,7 +135,7 @@ static int mga_vram_init(struct mga_device *mdev)
 	remove_conflicting_framebuffers(aper, "mgafb", true);
 	kfree(aper);
 
-	if (!request_mem_region(mdev->mc.vram_base, mdev->mc.vram_window,
+	if (!devm_request_mem_region(mdev->dev->dev, mdev->mc.vram_base, mdev->mc.vram_window,
 				"mgadrmfb_vram")) {
 		DRM_ERROR("can't reserve VRAM\n");
 		return -ENXIO;
@@ -177,13 +168,13 @@ static int mgag200_device_init(struct drm_device *dev,
 	mdev->rmmio_base = pci_resource_start(mdev->dev->pdev, 1);
 	mdev->rmmio_size = pci_resource_len(mdev->dev->pdev, 1);
 
-	if (!request_mem_region(mdev->rmmio_base, mdev->rmmio_size,
+	if (!devm_request_mem_region(mdev->dev->dev, mdev->rmmio_base, mdev->rmmio_size,
 				"mgadrmfb_mmio")) {
 		DRM_ERROR("can't reserve mmio registers\n");
 		return -ENOMEM;
 	}
 
-	mdev->rmmio = pci_iomap(dev->pdev, 1, 0);
+	mdev->rmmio = pcim_iomap(dev->pdev, 1, 0);
 	if (mdev->rmmio == NULL)
 		return -ENOMEM;
 
@@ -192,10 +183,8 @@ static int mgag200_device_init(struct drm_device *dev,
 		mdev->reg_1e24 = RREG32(0x1e24);
 
 	ret = mga_vram_init(mdev);
-	if (ret) {
-		release_mem_region(mdev->rmmio_base, mdev->rmmio_size);
+	if (ret)
 		return ret;
-	}
 
 	mdev->bpp_shifts[0] = 0;
 	mdev->bpp_shifts[1] = 1;
@@ -204,12 +193,6 @@ static int mgag200_device_init(struct drm_device *dev,
 	return 0;
 }
 
-void mgag200_device_fini(struct mga_device *mdev)
-{
-	release_mem_region(mdev->rmmio_base, mdev->rmmio_size);
-	mga_vram_fini(mdev);
-}
-
 /*
  * Functions here will be called by the core once it's bound the driver to
  * a PCI device
@@ -221,7 +204,7 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 	struct mga_device *mdev;
 	int r;
 
-	mdev = kzalloc(sizeof(struct mga_device), GFP_KERNEL);
+	mdev = devm_kzalloc(dev->dev, sizeof(struct mga_device), GFP_KERNEL);
 	if (mdev == NULL)
 		return -ENOMEM;
 	dev->dev_private = (void *)mdev;
@@ -265,8 +248,6 @@ int mgag200_driver_unload(struct drm_device *dev)
 	mgag200_fbdev_fini(mdev);
 	drm_mode_config_cleanup(dev);
 	mgag200_mm_fini(mdev);
-	mgag200_device_fini(mdev);
-	kfree(mdev);
 	dev->dev_private = NULL;
 	return 0;
 }
-- 
1.7.12.4

[PATCH 0/3] mgag200: bug fix, cleanup and managed devices

[Christopher Harvey]

Patch one is a simple bug fix for G200ER cards.

Patch two doesn't change any code

Patch three makes the mgag200 driver use managed devices where
possible. I'm not too sure how to test this one properly, other than
to just boot a bunch of boards and stare at the code changes for a
long while. Any testing hints are appreciated.

Christopher Harvey (3):
  drm/mgag200: Index 24 in extended CRTC registers is 24 in hex, not decimal.
  drm: Misc comment cleanup
  drm/mgag200: Convert to managed device resources where possible

 drivers/gpu/drm/mgag200/mgag200_drv.h  |  6 +++---
 drivers/gpu/drm/mgag200/mgag200_fb.c   |  9 +++------
 drivers/gpu/drm/mgag200/mgag200_main.c | 29 +++++------------------------
 drivers/gpu/drm/mgag200/mgag200_mode.c | 13 +++----------
 include/drm/drm_crtc.h                 |  2 +-
 include/uapi/drm/drm_mode.h            |  6 +++---
 6 files changed, 18 insertions(+), 47 deletions(-)

-- 
1.7.12.4

Re: [PATCH 1/3] drm/mgag200: Index 24 in extended CRTC registers is 24 in hex, not decimal.

[Mathieu Larouche]

On 05/04/2013 10:51 AM, Christopher Harvey wrote:
> This change properly enables the "requester" in G200ER cards that is
> responsible for getting pixels out of memory and clocking them out to
> the screen.
>
> Signed-off-by: Christopher Harvey <charvey@matrox.com>

Signed-off-by: Mathieu Larouche <mathieu.larouche@matrox.com>

> ---
>   drivers/gpu/drm/mgag200/mgag200_mode.c | 13 +++----------
>   1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 7337013..f988965 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -751,8 +751,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   	int i;
>   	unsigned char misc = 0;
>   	unsigned char ext_vga[6];
> -	unsigned char ext_vga_index24;
> -	unsigned char dac_index90 = 0;
>   	u8 bppshift;
>   
>   	static unsigned char dacvalue[] = {
> @@ -803,7 +801,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   		option2 = 0x0000b000;
>   		break;
>   	case G200_ER:
> -		dac_index90 = 0;
>   		break;
>   	}
>   
> @@ -852,10 +849,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   		WREG_DAC(i, dacvalue[i]);
>   	}
>   
> -	if (mdev->type == G200_ER) {
> -		WREG_DAC(0x90, dac_index90);
> -	}
> -
> +	if (mdev->type == G200_ER)
> +		WREG_DAC(0x90, 0);
>   
>   	if (option)
>   		pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option);
> @@ -952,8 +947,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   	if (mdev->type == G200_WB)
>   		ext_vga[1] |= 0x88;
>   
> -	ext_vga_index24 = 0x05;
> -
>   	/* Set pixel clocks */
>   	misc = 0x2d;
>   	WREG8(MGA_MISC_OUT, misc);
> @@ -965,7 +958,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>   	}
>   
>   	if (mdev->type == G200_ER)
> -		WREG_ECRT(24, ext_vga_index24);
> +		WREG_ECRT(0x24, 0x5);
>   
>   	if (mdev->type == G200_EV) {
>   		WREG_ECRT(6, 0);

-- 
Mathieu Larouche Ing./Eng.
Software Designer
Matrox Graphics Inc.
Phone : 514 822-6000 x7905
Email : mathieu.larouche@matrox.com