All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] i915 fixes for sparse warnings.
@ 2012-04-16 21:07 Ben Widawsky
  2012-04-16 21:07 ` [PATCH 1/9] drm/i915: [sparse] trivial sparse fixes Ben Widawsky
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Patch 1/2 is a mix of fairly trivial stuff. I see no reason to not take
these.
Patch 3-9 Are all __iomem related. These should run through some
regression testing.

Most of the __iomem stuff fell into place fairly easily, but it's
definitely subject to a lot of copy/paste errors, since trying to
automate the replacement didn't workout well. Throughout the patches, I
tried really hard to not force cast things, but a couple of situations
cropped up which gave me no choice. What I hope to achieve by that is
there should be no functional or performance changes (aside from
copy/paste).

I've given up on fixing intel_bios.c completely.

These are based off of dinq as of yesterday.


Ben Widawsky (9):
  drm/i915: [sparse] trivial sparse fixes
  drm/i915: [sparse] don't use variable size arrays
  drm/i915: [sparse] __iomem fixes for opregion
  drm/i915: [sparse] __iomem fixes for overlay
  drm/i915: [sparse] __iomem fixes for debugfs
  drm/i915: [sparse] __iomem fixes for ringbuffer
  drm/i915: [sparse] forced __iomem ringbuffer fixes
  drm/i915: [sparse] __iomem fixes for gem
  drm/i915: [sparse] __iomem fixes for intel_bios

 drivers/gpu/drm/i915/i915_debugfs.c      |   28 ++++---
 drivers/gpu/drm/i915/i915_drv.h          |   12 +--
 drivers/gpu/drm/i915/i915_gem.c          |   11 +--
 drivers/gpu/drm/i915/i915_ioc32.c        |    5 +-
 drivers/gpu/drm/i915/i915_trace_points.c |    2 +
 drivers/gpu/drm/i915/intel_acpi.c        |    1 +
 drivers/gpu/drm/i915/intel_bios.c        |   20 +++--
 drivers/gpu/drm/i915/intel_display.c     |    2 +-
 drivers/gpu/drm/i915/intel_fb.c          |    2 +-
 drivers/gpu/drm/i915/intel_opregion.c    |   67 ++++++++-------
 drivers/gpu/drm/i915/intel_overlay.c     |  130 +++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_ringbuffer.c  |   28 ++++---
 drivers/gpu/drm/i915/intel_sdvo.c        |   26 ++++--
 13 files changed, 197 insertions(+), 137 deletions(-)

-- 
1.7.10

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/9] drm/i915: [sparse] trivial sparse fixes
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
@ 2012-04-16 21:07 ` Ben Widawsky
  2012-04-16 21:13   ` Chris Wilson
  2012-04-16 21:07 ` [PATCH 2/9] drm/i915: [sparse] don't use variable size arrays Ben Widawsky
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

This should contain all the changes which require no thought to make
sparse happy.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c      |    2 +-
 drivers/gpu/drm/i915/i915_drv.h          |    2 ++
 drivers/gpu/drm/i915/i915_ioc32.c        |    5 ++++-
 drivers/gpu/drm/i915/i915_trace_points.c |    2 ++
 drivers/gpu/drm/i915/intel_acpi.c        |    1 +
 drivers/gpu/drm/i915/intel_display.c     |    2 +-
 drivers/gpu/drm/i915/intel_fb.c          |    2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c  |    4 ++--
 drivers/gpu/drm/i915/intel_sdvo.c        |    2 +-
 9 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 97c9630..35462df 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1832,7 +1832,7 @@ static int i915_forcewake_open(struct inode *inode, struct file *file)
 	return 0;
 }
 
-int i915_forcewake_release(struct inode *inode, struct file *file)
+static int i915_forcewake_release(struct inode *inode, struct file *file)
 {
 	struct drm_device *dev = inode->i_private;
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 422f424..303cee7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1128,8 +1128,10 @@ extern void i915_driver_preclose(struct drm_device *dev,
 extern void i915_driver_postclose(struct drm_device *dev,
 				  struct drm_file *file_priv);
 extern int i915_driver_device_is_agp(struct drm_device * dev);
+#ifdef CONFIG_COMPAT
 extern long i915_compat_ioctl(struct file *filp, unsigned int cmd,
 			      unsigned long arg);
+#endif
 extern int i915_emit_box(struct drm_device *dev,
 			 struct drm_clip_rect *box,
 			 int DR1, int DR4);
diff --git a/drivers/gpu/drm/i915/i915_ioc32.c b/drivers/gpu/drm/i915/i915_ioc32.c
index 13b0289..0e72abb 100644
--- a/drivers/gpu/drm/i915/i915_ioc32.c
+++ b/drivers/gpu/drm/i915/i915_ioc32.c
@@ -34,6 +34,7 @@
 #include "drmP.h"
 #include "drm.h"
 #include "i915_drm.h"
+#include "i915_drv.h"
 
 typedef struct _drm_i915_batchbuffer32 {
 	int start;		/* agp offset */
@@ -181,7 +182,7 @@ static int compat_i915_alloc(struct file *file, unsigned int cmd,
 			 (unsigned long)request);
 }
 
-drm_ioctl_compat_t *i915_compat_ioctls[] = {
+static drm_ioctl_compat_t *i915_compat_ioctls[] = {
 	[DRM_I915_BATCHBUFFER] = compat_i915_batchbuffer,
 	[DRM_I915_CMDBUFFER] = compat_i915_cmdbuffer,
 	[DRM_I915_GETPARAM] = compat_i915_getparam,
@@ -189,6 +190,7 @@ drm_ioctl_compat_t *i915_compat_ioctls[] = {
 	[DRM_I915_ALLOC] = compat_i915_alloc
 };
 
+#ifdef CONFIG_COMPAT
 /**
  * Called whenever a 32-bit process running under a 64-bit kernel
  * performs an ioctl on /dev/dri/card<n>.
@@ -217,3 +219,4 @@ long i915_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 
 	return ret;
 }
+#endif
diff --git a/drivers/gpu/drm/i915/i915_trace_points.c b/drivers/gpu/drm/i915/i915_trace_points.c
index ead876e..f1df2bd 100644
--- a/drivers/gpu/drm/i915/i915_trace_points.c
+++ b/drivers/gpu/drm/i915/i915_trace_points.c
@@ -7,5 +7,7 @@
 
 #include "i915_drv.h"
 
+#ifndef __CHECKER__
 #define CREATE_TRACE_POINTS
 #include "i915_trace.h"
+#endif
diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
index f152b2a..f413899 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -9,6 +9,7 @@
 #include <acpi/acpi_drivers.h>
 
 #include "drmP.h"
+#include "i915_drv.h"
 
 #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 33aaad3..c457716 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9460,7 +9460,7 @@ struct intel_quirk {
 	void (*hook)(struct drm_device *dev);
 };
 
-struct intel_quirk intel_quirks[] = {
+static struct intel_quirk intel_quirks[] = {
 	/* HP Mini needs pipe A force quirk (LP: #322104) */
 	{ 0x27ae, 0x103c, 0x361a, quirk_pipea_force },
 
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 19ecd78..71ef289 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -94,7 +94,7 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
 	mutex_lock(&dev->struct_mutex);
 
 	/* Flush everything out, we'll be doing GTT only from now on */
-	ret = intel_pin_and_fence_fb_obj(dev, obj, false);
+	ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
 	if (ret) {
 		DRM_ERROR("failed to pin fb: %d\n", ret);
 		goto out_unref;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6c14457..caec699 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -891,8 +891,8 @@ err:
 	return ret;
 }
 
-int intel_init_ring_buffer(struct drm_device *dev,
-			   struct intel_ring_buffer *ring)
+static int intel_init_ring_buffer(struct drm_device *dev,
+				  struct intel_ring_buffer *ring)
 {
 	struct drm_i915_gem_object *obj;
 	int ret;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 6898145..aa1a0d5 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1258,7 +1258,7 @@ intel_sdvo_get_analog_edid(struct drm_connector *connector)
 						    dev_priv->crt_ddc_pin));
 }
 
-enum drm_connector_status
+static enum drm_connector_status
 intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
 {
 	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/9] drm/i915: [sparse] don't use variable size arrays
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
  2012-04-16 21:07 ` [PATCH 1/9] drm/i915: [sparse] trivial sparse fixes Ben Widawsky
@ 2012-04-16 21:07 ` Ben Widawsky
  2012-04-18  1:39   ` Paulo Zanoni
  2012-04-16 21:07 ` [PATCH 3/9] drm/i915: [sparse] __iomem fixes for opregion Ben Widawsky
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

Sparse doesn't like:
"error: bad constant expression"

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c |   24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index aa1a0d5..1daa198 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -443,9 +443,17 @@ static const char *cmd_status_names[] = {
 static bool intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd,
 				 const void *args, int args_len)
 {
-	u8 buf[args_len*2 + 2], status;
-	struct i2c_msg msgs[args_len + 3];
-	int i, ret;
+	u8 *buf, status;
+	struct i2c_msg *msgs;
+	int i, ret = true;
+
+	buf = (u8 *)kzalloc(args_len * 2 + 2, GFP_KERNEL);
+	if (!buf)
+		return false;
+
+	msgs = drm_malloc_ab(args_len + 3, sizeof(*msgs));
+	if (!msgs)
+		return false;
 
 	intel_sdvo_debug_write(intel_sdvo, cmd, args, args_len);
 
@@ -479,15 +487,19 @@ static bool intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd,
 	ret = i2c_transfer(intel_sdvo->i2c, msgs, i+3);
 	if (ret < 0) {
 		DRM_DEBUG_KMS("I2c transfer returned %d\n", ret);
-		return false;
+		ret = false;
+		goto out;
 	}
 	if (ret != i+3) {
 		/* failure in I2C transfer */
 		DRM_DEBUG_KMS("I2c transfer returned %d/%d\n", ret, i+3);
-		return false;
+		ret = false;
 	}
 
-	return true;
+out:
+	drm_free_large(msgs);
+	kfree(buf);
+	return ret;
 }
 
 static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/9] drm/i915: [sparse] __iomem fixes for opregion
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
  2012-04-16 21:07 ` [PATCH 1/9] drm/i915: [sparse] trivial sparse fixes Ben Widawsky
  2012-04-16 21:07 ` [PATCH 2/9] drm/i915: [sparse] don't use variable size arrays Ben Widawsky
@ 2012-04-16 21:07 ` Ben Widawsky
  2012-04-16 21:07 ` [PATCH 4/9] drm/i915: [sparse] __iomem fixes for overlay Ben Widawsky
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

Almost all of the errors related __iomem problems.

Most of the changes here are trivial, however there is plenty of chance
for yank/paste errors.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |   10 ++---
 drivers/gpu/drm/i915/intel_opregion.c |   67 +++++++++++++++++++--------------
 2 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 303cee7..1ca8d8d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -122,11 +122,11 @@ struct opregion_asle;
 struct drm_i915_private;
 
 struct intel_opregion {
-	struct opregion_header *header;
-	struct opregion_acpi *acpi;
-	struct opregion_swsci *swsci;
-	struct opregion_asle *asle;
-	void *vbt;
+	struct opregion_header __iomem *header;
+	struct opregion_acpi __iomem *acpi;
+	struct opregion_swsci __iomem *swsci;
+	struct opregion_asle __iomem *asle;
+	void __iomem *vbt;
 	u32 __iomem *lid_state;
 };
 #define OPREGION_SIZE            (8*1024)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 34929ae..18bd0af 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -151,7 +151,7 @@ struct opregion_asle {
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct opregion_asle *asle = dev_priv->opregion.asle;
+	struct opregion_asle __iomem *asle = dev_priv->opregion.asle;
 	u32 max;
 
 	if (!(bclp & ASLE_BCLP_VALID))
@@ -163,7 +163,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 
 	max = intel_panel_get_max_backlight(dev);
 	intel_panel_set_backlight(dev, bclp * max / 255);
-	asle->cblv = (bclp*0x64)/0xff | ASLE_CBLV_VALID;
+	iowrite32((bclp*0x64)/0xff | ASLE_CBLV_VALID, &asle->cblv);
 
 	return 0;
 }
@@ -200,14 +200,14 @@ static u32 asle_set_pfit(struct drm_device *dev, u32 pfit)
 void intel_opregion_asle_intr(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct opregion_asle *asle = dev_priv->opregion.asle;
+	struct opregion_asle __iomem *asle = dev_priv->opregion.asle;
 	u32 asle_stat = 0;
 	u32 asle_req;
 
 	if (!asle)
 		return;
 
-	asle_req = asle->aslc & ASLE_REQ_MSK;
+	asle_req = ioread32(&asle->aslc) & ASLE_REQ_MSK;
 
 	if (!asle_req) {
 		DRM_DEBUG_DRIVER("non asle set request??\n");
@@ -215,31 +215,31 @@ void intel_opregion_asle_intr(struct drm_device *dev)
 	}
 
 	if (asle_req & ASLE_SET_ALS_ILLUM)
-		asle_stat |= asle_set_als_illum(dev, asle->alsi);
+		asle_stat |= asle_set_als_illum(dev, ioread32(&asle->alsi));
 
 	if (asle_req & ASLE_SET_BACKLIGHT)
-		asle_stat |= asle_set_backlight(dev, asle->bclp);
+		asle_stat |= asle_set_backlight(dev, ioread32(&asle->bclp));
 
 	if (asle_req & ASLE_SET_PFIT)
-		asle_stat |= asle_set_pfit(dev, asle->pfit);
+		asle_stat |= asle_set_pfit(dev, ioread32(&asle->pfit));
 
 	if (asle_req & ASLE_SET_PWM_FREQ)
-		asle_stat |= asle_set_pwm_freq(dev, asle->pfmb);
+		asle_stat |= asle_set_pwm_freq(dev, ioread32(&asle->pfmb));
 
-	asle->aslc = asle_stat;
+	iowrite32(asle_stat, &asle->aslc);
 }
 
 void intel_opregion_gse_intr(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct opregion_asle *asle = dev_priv->opregion.asle;
+	struct opregion_asle __iomem *asle = dev_priv->opregion.asle;
 	u32 asle_stat = 0;
 	u32 asle_req;
 
 	if (!asle)
 		return;
 
-	asle_req = asle->aslc & ASLE_REQ_MSK;
+	asle_req = ioread32(&asle->aslc) & ASLE_REQ_MSK;
 
 	if (!asle_req) {
 		DRM_DEBUG_DRIVER("non asle set request??\n");
@@ -252,7 +252,7 @@ void intel_opregion_gse_intr(struct drm_device *dev)
 	}
 
 	if (asle_req & ASLE_SET_BACKLIGHT)
-		asle_stat |= asle_set_backlight(dev, asle->bclp);
+		asle_stat |= asle_set_backlight(dev, ioread32(&asle->bclp));
 
 	if (asle_req & ASLE_SET_PFIT) {
 		DRM_DEBUG_DRIVER("Pfit is not supported\n");
@@ -264,7 +264,7 @@ void intel_opregion_gse_intr(struct drm_device *dev)
 		asle_stat |= ASLE_PWM_FREQ_FAILED;
 	}
 
-	asle->aslc = asle_stat;
+	iowrite32(asle_stat, &asle->aslc);
 }
 #define ASLE_ALS_EN    (1<<0)
 #define ASLE_BLC_EN    (1<<1)
@@ -274,15 +274,16 @@ void intel_opregion_gse_intr(struct drm_device *dev)
 void intel_opregion_enable_asle(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct opregion_asle *asle = dev_priv->opregion.asle;
+	struct opregion_asle __iomem *asle = dev_priv->opregion.asle;
 
 	if (asle) {
 		if (IS_MOBILE(dev))
 			intel_enable_asle(dev);
 
-		asle->tche = ASLE_ALS_EN | ASLE_BLC_EN | ASLE_PFIT_EN |
-			ASLE_PFMB_EN;
-		asle->ardy = 1;
+		iowrite32(ASLE_ALS_EN | ASLE_BLC_EN | ASLE_PFIT_EN |
+			  ASLE_PFMB_EN,
+			  &asle->tche);
+		iowrite32(1, &asle->ardy);
 	}
 }
 
@@ -300,7 +301,7 @@ static int intel_opregion_video_event(struct notifier_block *nb,
 	   Linux, these are handled by the dock, button and video drivers.
 	*/
 
-	struct opregion_acpi *acpi;
+	struct opregion_acpi __iomem *acpi;
 	struct acpi_bus_event *event = data;
 	int ret = NOTIFY_OK;
 
@@ -312,10 +313,11 @@ static int intel_opregion_video_event(struct notifier_block *nb,
 
 	acpi = system_opregion->acpi;
 
-	if (event->type == 0x80 && !(acpi->cevt & 0x1))
+	if (event->type == 0x80 &&
+	    (ioread32(&acpi->cevt) & 1) == 0)
 		ret = NOTIFY_BAD;
 
-	acpi->csts = 0;
+	iowrite32(0, &acpi->csts);
 
 	return ret;
 }
@@ -339,6 +341,7 @@ static void intel_didl_outputs(struct drm_device *dev)
 	struct acpi_device *acpi_dev, *acpi_cdev, *acpi_video_bus = NULL;
 	unsigned long long device_id;
 	acpi_status status;
+	u32 temp;
 	int i = 0;
 
 	handle = DEVICE_ACPI_HANDLE(&dev->pdev->dev);
@@ -373,7 +376,8 @@ static void intel_didl_outputs(struct drm_device *dev)
 		if (ACPI_SUCCESS(status)) {
 			if (!device_id)
 				goto blind_set;
-			opregion->acpi->didl[i] = (u32)(device_id & 0x0f0f);
+			iowrite32((u32)(device_id & 0x0f0f),
+				  &opregion->acpi->didl[i]);
 			i++;
 		}
 	}
@@ -381,7 +385,7 @@ static void intel_didl_outputs(struct drm_device *dev)
 end:
 	/* If fewer than 8 outputs, the list must be null terminated */
 	if (i < 8)
-		opregion->acpi->didl[i] = 0;
+		iowrite32(0, &opregion->acpi->didl[i]);
 	return;
 
 blind_set:
@@ -415,7 +419,9 @@ blind_set:
 			output_type = ACPI_LVDS_OUTPUT;
 			break;
 		}
-		opregion->acpi->didl[i] |= (1<<31) | output_type | i;
+		temp = ioread32(&opregion->acpi->didl[i]);
+		iowrite32(temp | (1<<31) | output_type | i,
+			  &opregion->acpi->didl[i]);
 		i++;
 	}
 	goto end;
@@ -436,8 +442,8 @@ void intel_opregion_init(struct drm_device *dev)
 		/* Notify BIOS we are ready to handle ACPI video ext notifs.
 		 * Right now, all the events are handled by the ACPI video module.
 		 * We don't actually need to do anything with them. */
-		opregion->acpi->csts = 0;
-		opregion->acpi->drdy = 1;
+		iowrite32(0, &opregion->acpi->csts);
+		iowrite32(1, &opregion->acpi->drdy);
 
 		system_opregion = opregion;
 		register_acpi_notifier(&intel_opregion_notifier);
@@ -456,7 +462,7 @@ void intel_opregion_fini(struct drm_device *dev)
 		return;
 
 	if (opregion->acpi) {
-		opregion->acpi->drdy = 0;
+		iowrite32(0, &opregion->acpi->drdy);
 
 		system_opregion = NULL;
 		unregister_acpi_notifier(&intel_opregion_notifier);
@@ -476,8 +482,9 @@ int intel_opregion_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_opregion *opregion = &dev_priv->opregion;
-	void *base;
+	void __iomem *base;
 	u32 asls, mboxes;
+	char buf[sizeof(OPREGION_SIGNATURE)];
 	int err = 0;
 
 	pci_read_config_dword(dev->pdev, PCI_ASLS, &asls);
@@ -491,7 +498,9 @@ int intel_opregion_setup(struct drm_device *dev)
 	if (!base)
 		return -ENOMEM;
 
-	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
+	memcpy_fromio(buf, base, sizeof(buf));
+
+	if (memcmp(buf, OPREGION_SIGNATURE, 16)) {
 		DRM_DEBUG_DRIVER("opregion signature mismatch\n");
 		err = -EINVAL;
 		goto err_out;
@@ -501,7 +510,7 @@ int intel_opregion_setup(struct drm_device *dev)
 
 	opregion->lid_state = base + ACPI_CLID;
 
-	mboxes = opregion->header->mboxes;
+	mboxes = ioread32(&opregion->header->mboxes);
 	if (mboxes & MBOX_ACPI) {
 		DRM_DEBUG_DRIVER("Public ACPI methods supported\n");
 		opregion->acpi = base + OPREGION_ACPI_OFFSET;
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/9] drm/i915: [sparse] __iomem fixes for overlay
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
                   ` (2 preceding siblings ...)
  2012-04-16 21:07 ` [PATCH 3/9] drm/i915: [sparse] __iomem fixes for opregion Ben Widawsky
@ 2012-04-16 21:07 ` Ben Widawsky
  2012-04-16 21:07 ` [PATCH 5/9] drm/i915: [sparse] __iomem fixes for debugfs Ben Widawsky
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

With the exception of a forced cast for phys_obj stuff (a problem in
other patches as well) all of these are fairly simple __iomem compliance
fixes.

As with other patches, yank/paste errors may exist.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/intel_overlay.c |  130 +++++++++++++++++++---------------
 1 file changed, 72 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 80b331c..95cc21a 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -187,14 +187,14 @@ struct intel_overlay {
 	void (*flip_tail)(struct intel_overlay *);
 };
 
-static struct overlay_registers *
+static struct overlay_registers __iomem *
 intel_overlay_map_regs(struct intel_overlay *overlay)
 {
 	drm_i915_private_t *dev_priv = overlay->dev->dev_private;
-	struct overlay_registers *regs;
+	struct overlay_registers __iomem *regs;
 
 	if (OVERLAY_NEEDS_PHYSICAL(overlay->dev))
-		regs = overlay->reg_bo->phys_obj->handle->vaddr;
+		regs = (struct overlay_registers __iomem *)overlay->reg_bo->phys_obj->handle->vaddr;
 	else
 		regs = io_mapping_map_wc(dev_priv->mm.gtt_mapping,
 					 overlay->reg_bo->gtt_offset);
@@ -203,7 +203,7 @@ intel_overlay_map_regs(struct intel_overlay *overlay)
 }
 
 static void intel_overlay_unmap_regs(struct intel_overlay *overlay,
-				     struct overlay_registers *regs)
+				     struct overlay_registers __iomem *regs)
 {
 	if (!OVERLAY_NEEDS_PHYSICAL(overlay->dev))
 		io_mapping_unmap(regs);
@@ -619,14 +619,15 @@ static const u16 uv_static_hcoeffs[N_HORIZ_UV_TAPS * N_PHASES] = {
 	0x3000, 0x0800, 0x3000
 };
 
-static void update_polyphase_filter(struct overlay_registers *regs)
+static void update_polyphase_filter(struct overlay_registers __iomem *regs)
 {
-	memcpy(regs->Y_HCOEFS, y_static_hcoeffs, sizeof(y_static_hcoeffs));
-	memcpy(regs->UV_HCOEFS, uv_static_hcoeffs, sizeof(uv_static_hcoeffs));
+	memcpy_toio(regs->Y_HCOEFS, y_static_hcoeffs, sizeof(y_static_hcoeffs));
+	memcpy_toio(regs->UV_HCOEFS, uv_static_hcoeffs,
+		    sizeof(uv_static_hcoeffs));
 }
 
 static bool update_scaling_factors(struct intel_overlay *overlay,
-				   struct overlay_registers *regs,
+				   struct overlay_registers __iomem *regs,
 				   struct put_image_params *params)
 {
 	/* fixed point with a 12 bit shift */
@@ -665,16 +666,19 @@ static bool update_scaling_factors(struct intel_overlay *overlay,
 	overlay->old_xscale = xscale;
 	overlay->old_yscale = yscale;
 
-	regs->YRGBSCALE = (((yscale & FRACT_MASK) << 20) |
-			   ((xscale >> FP_SHIFT)  << 16) |
-			   ((xscale & FRACT_MASK) << 3));
+	iowrite32(((yscale & FRACT_MASK) << 20) |
+		  ((xscale >> FP_SHIFT)  << 16) |
+		  ((xscale & FRACT_MASK) << 3),
+		 &regs->YRGBSCALE);
 
-	regs->UVSCALE = (((yscale_UV & FRACT_MASK) << 20) |
-			 ((xscale_UV >> FP_SHIFT)  << 16) |
-			 ((xscale_UV & FRACT_MASK) << 3));
+	iowrite32(((yscale_UV & FRACT_MASK) << 20) |
+		  ((xscale_UV >> FP_SHIFT)  << 16) |
+		  ((xscale_UV & FRACT_MASK) << 3),
+		 &regs->UVSCALE);
 
-	regs->UVSCALEV = ((((yscale    >> FP_SHIFT) << 16) |
-			   ((yscale_UV >> FP_SHIFT) << 0)));
+	iowrite32((((yscale    >> FP_SHIFT) << 16) |
+		   ((yscale_UV >> FP_SHIFT) << 0)),
+		 &regs->UVSCALEV);
 
 	if (scale_changed)
 		update_polyphase_filter(regs);
@@ -683,30 +687,32 @@ static bool update_scaling_factors(struct intel_overlay *overlay,
 }
 
 static void update_colorkey(struct intel_overlay *overlay,
-			    struct overlay_registers *regs)
+			    struct overlay_registers __iomem *regs)
 {
 	u32 key = overlay->color_key;
 
 	switch (overlay->crtc->base.fb->bits_per_pixel) {
 	case 8:
-		regs->DCLRKV = 0;
-		regs->DCLRKM = CLK_RGB8I_MASK | DST_KEY_ENABLE;
+		iowrite32(0, &regs->DCLRKV);
+		iowrite32(CLK_RGB8I_MASK | DST_KEY_ENABLE, &regs->DCLRKM);
 		break;
 
 	case 16:
 		if (overlay->crtc->base.fb->depth == 15) {
-			regs->DCLRKV = RGB15_TO_COLORKEY(key);
-			regs->DCLRKM = CLK_RGB15_MASK | DST_KEY_ENABLE;
+			iowrite32(RGB15_TO_COLORKEY(key), &regs->DCLRKV);
+			iowrite32(CLK_RGB15_MASK | DST_KEY_ENABLE,
+				  &regs->DCLRKM);
 		} else {
-			regs->DCLRKV = RGB16_TO_COLORKEY(key);
-			regs->DCLRKM = CLK_RGB16_MASK | DST_KEY_ENABLE;
+			iowrite32(RGB16_TO_COLORKEY(key), &regs->DCLRKV);
+			iowrite32(CLK_RGB16_MASK | DST_KEY_ENABLE,
+				  &regs->DCLRKM);
 		}
 		break;
 
 	case 24:
 	case 32:
-		regs->DCLRKV = key;
-		regs->DCLRKM = CLK_RGB24_MASK | DST_KEY_ENABLE;
+		iowrite32(key, &regs->DCLRKV);
+		iowrite32(CLK_RGB24_MASK | DST_KEY_ENABLE, &regs->DCLRKM);
 		break;
 	}
 }
@@ -761,9 +767,10 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 				      struct put_image_params *params)
 {
 	int ret, tmp_width;
-	struct overlay_registers *regs;
+	struct overlay_registers __iomem *regs;
 	bool scale_changed = false;
 	struct drm_device *dev = overlay->dev;
+	u32 swidth, swidthsw, sheight, ostride;
 
 	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
 	BUG_ON(!mutex_is_locked(&dev->mode_config.mutex));
@@ -782,16 +789,18 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 		goto out_unpin;
 
 	if (!overlay->active) {
+		u32 oconfig;
 		regs = intel_overlay_map_regs(overlay);
 		if (!regs) {
 			ret = -ENOMEM;
 			goto out_unpin;
 		}
-		regs->OCONFIG = OCONF_CC_OUT_8BIT;
+		oconfig = OCONF_CC_OUT_8BIT;
 		if (IS_GEN4(overlay->dev))
-			regs->OCONFIG |= OCONF_CSC_MODE_BT709;
-		regs->OCONFIG |= overlay->crtc->pipe == 0 ?
+			oconfig |= OCONF_CSC_MODE_BT709;
+		oconfig |= overlay->crtc->pipe == 0 ?
 			OCONF_PIPE_A : OCONF_PIPE_B;
+		iowrite32(oconfig, &regs->OCONFIG);
 		intel_overlay_unmap_regs(overlay, regs);
 
 		ret = intel_overlay_on(overlay);
@@ -805,42 +814,46 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 		goto out_unpin;
 	}
 
-	regs->DWINPOS = (params->dst_y << 16) | params->dst_x;
-	regs->DWINSZ = (params->dst_h << 16) | params->dst_w;
+	iowrite32((params->dst_y << 16) | params->dst_x, &regs->DWINPOS);
+	iowrite32((params->dst_h << 16) | params->dst_w, &regs->DWINSZ);
 
 	if (params->format & I915_OVERLAY_YUV_PACKED)
 		tmp_width = packed_width_bytes(params->format, params->src_w);
 	else
 		tmp_width = params->src_w;
 
-	regs->SWIDTH = params->src_w;
-	regs->SWIDTHSW = calc_swidthsw(overlay->dev,
-				       params->offset_Y, tmp_width);
-	regs->SHEIGHT = params->src_h;
-	regs->OBUF_0Y = new_bo->gtt_offset + params->offset_Y;
-	regs->OSTRIDE = params->stride_Y;
+	swidth = params->src_w;
+	swidthsw = calc_swidthsw(overlay->dev, params->offset_Y, tmp_width);
+	sheight = params->src_h;
+	iowrite32(new_bo->gtt_offset + params->offset_Y, &regs->OBUF_0Y);
+	ostride = params->stride_Y;
 
 	if (params->format & I915_OVERLAY_YUV_PLANAR) {
 		int uv_hscale = uv_hsubsampling(params->format);
 		int uv_vscale = uv_vsubsampling(params->format);
 		u32 tmp_U, tmp_V;
-		regs->SWIDTH |= (params->src_w/uv_hscale) << 16;
+		swidth |= (params->src_w/uv_hscale) << 16;
 		tmp_U = calc_swidthsw(overlay->dev, params->offset_U,
 				      params->src_w/uv_hscale);
 		tmp_V = calc_swidthsw(overlay->dev, params->offset_V,
 				      params->src_w/uv_hscale);
-		regs->SWIDTHSW |= max_t(u32, tmp_U, tmp_V) << 16;
-		regs->SHEIGHT |= (params->src_h/uv_vscale) << 16;
-		regs->OBUF_0U = new_bo->gtt_offset + params->offset_U;
-		regs->OBUF_0V = new_bo->gtt_offset + params->offset_V;
-		regs->OSTRIDE |= params->stride_UV << 16;
+		swidthsw |= max_t(u32, tmp_U, tmp_V) << 16;
+		sheight |= (params->src_h/uv_vscale) << 16;
+		iowrite32(new_bo->gtt_offset + params->offset_U, &regs->OBUF_0U);
+		iowrite32(new_bo->gtt_offset + params->offset_V, &regs->OBUF_0V);
+		ostride |= params->stride_UV << 16;
 	}
 
+	iowrite32(swidth, &regs->SWIDTH);
+	iowrite32(swidthsw, &regs->SWIDTHSW);
+	iowrite32(sheight, &regs->SHEIGHT);
+	iowrite32(ostride, &regs->OSTRIDE);
+
 	scale_changed = update_scaling_factors(overlay, regs, params);
 
 	update_colorkey(overlay, regs);
 
-	regs->OCMD = overlay_cmd_reg(params);
+	iowrite32(overlay_cmd_reg(params), &regs->OCMD);
 
 	intel_overlay_unmap_regs(overlay, regs);
 
@@ -860,7 +873,7 @@ out_unpin:
 
 int intel_overlay_switch_off(struct intel_overlay *overlay)
 {
-	struct overlay_registers *regs;
+	struct overlay_registers __iomem *regs;
 	struct drm_device *dev = overlay->dev;
 	int ret;
 
@@ -879,7 +892,7 @@ int intel_overlay_switch_off(struct intel_overlay *overlay)
 		return ret;
 
 	regs = intel_overlay_map_regs(overlay);
-	regs->OCMD = 0;
+	iowrite32(0, &regs->OCMD);
 	intel_overlay_unmap_regs(overlay, regs);
 
 	ret = intel_overlay_off(overlay);
@@ -1250,10 +1263,11 @@ out_free:
 }
 
 static void update_reg_attrs(struct intel_overlay *overlay,
-			     struct overlay_registers *regs)
+			     struct overlay_registers __iomem *regs)
 {
-	regs->OCLRC0 = (overlay->contrast << 18) | (overlay->brightness & 0xff);
-	regs->OCLRC1 = overlay->saturation;
+	iowrite32((overlay->contrast << 18) | (overlay->brightness & 0xff),
+		  &regs->OCLRC0);
+	iowrite32(overlay->saturation, &regs->OCLRC1);
 }
 
 static bool check_gamma_bounds(u32 gamma1, u32 gamma2)
@@ -1306,7 +1320,7 @@ int intel_overlay_attrs(struct drm_device *dev, void *data,
 	struct drm_intel_overlay_attrs *attrs = data;
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_overlay *overlay;
-	struct overlay_registers *regs;
+	struct overlay_registers __iomem *regs;
 	int ret;
 
 	if (!dev_priv) {
@@ -1396,7 +1410,7 @@ void intel_setup_overlay(struct drm_device *dev)
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_overlay *overlay;
 	struct drm_i915_gem_object *reg_bo;
-	struct overlay_registers *regs;
+	struct overlay_registers __iomem *regs;
 	int ret;
 
 	if (!HAS_OVERLAY(dev))
@@ -1451,7 +1465,7 @@ void intel_setup_overlay(struct drm_device *dev)
 	if (!regs)
 		goto out_unpin_bo;
 
-	memset(regs, 0, sizeof(struct overlay_registers));
+	memset_io(regs, 0, sizeof(struct overlay_registers));
 	update_polyphase_filter(regs);
 	update_reg_attrs(overlay, regs);
 
@@ -1499,14 +1513,14 @@ struct intel_overlay_error_state {
 	u32 isr;
 };
 
-static struct overlay_registers *
+static struct overlay_registers __iomem *
 intel_overlay_map_regs_atomic(struct intel_overlay *overlay)
 {
 	drm_i915_private_t *dev_priv = overlay->dev->dev_private;
-	struct overlay_registers *regs;
+	struct overlay_registers __iomem *regs;
 
 	if (OVERLAY_NEEDS_PHYSICAL(overlay->dev))
-		regs = overlay->reg_bo->phys_obj->handle->vaddr;
+		regs = (struct overlay_registers __iomem *)overlay->reg_bo->phys_obj->handle->vaddr;
 	else
 		regs = io_mapping_map_atomic_wc(dev_priv->mm.gtt_mapping,
 						overlay->reg_bo->gtt_offset);
@@ -1515,7 +1529,7 @@ intel_overlay_map_regs_atomic(struct intel_overlay *overlay)
 }
 
 static void intel_overlay_unmap_regs_atomic(struct intel_overlay *overlay,
-					    struct overlay_registers *regs)
+					struct overlay_registers __iomem *regs)
 {
 	if (!OVERLAY_NEEDS_PHYSICAL(overlay->dev))
 		io_mapping_unmap_atomic(regs);
@@ -1540,9 +1554,9 @@ intel_overlay_capture_error_state(struct drm_device *dev)
 	error->dovsta = I915_READ(DOVSTA);
 	error->isr = I915_READ(ISR);
 	if (OVERLAY_NEEDS_PHYSICAL(overlay->dev))
-		error->base = (long) overlay->reg_bo->phys_obj->handle->vaddr;
+		error->base = (__force long)overlay->reg_bo->phys_obj->handle->vaddr;
 	else
-		error->base = (long) overlay->reg_bo->gtt_offset;
+		error->base = overlay->reg_bo->gtt_offset;
 
 	regs = intel_overlay_map_regs_atomic(overlay);
 	if (!regs)
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/9] drm/i915: [sparse] __iomem fixes for debugfs
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
                   ` (3 preceding siblings ...)
  2012-04-16 21:07 ` [PATCH 4/9] drm/i915: [sparse] __iomem fixes for overlay Ben Widawsky
@ 2012-04-16 21:07 ` Ben Widawsky
  2012-04-16 21:07 ` [PATCH 6/9] drm/i915: [sparse] __iomem fixes for ringbuffer Ben Widawsky
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

These were mostly straight forward. No forced casting needed.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 35462df..cfe3c82 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -586,18 +586,18 @@ static int i915_hws_info(struct seq_file *m, void *data)
 	struct drm_device *dev = node->minor->dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_ring_buffer *ring;
-	const volatile u32 __iomem *hws;
+	void __iomem *hws;
 	int i;
 
 	ring = &dev_priv->ring[(uintptr_t)node->info_ent->data];
-	hws = (volatile u32 __iomem *)ring->status_page.page_addr;
+	hws = (void __iomem *)ring->status_page.page_addr;
 	if (hws == NULL)
 		return 0;
 
 	for (i = 0; i < 4096 / sizeof(u32) / 4; i += 4) {
 		seq_printf(m, "0x%08x: 0x%08x 0x%08x 0x%08x 0x%08x\n",
-			   i * 4,
-			   hws[i], hws[i + 1], hws[i + 2], hws[i + 3]);
+			   i * 4, ioread32(hws), ioread32(hws + 4),
+			   ioread32(hws + 8), ioread32(hws + 0xc));
 	}
 	return 0;
 }
@@ -622,8 +622,8 @@ static int i915_ringbuffer_data(struct seq_file *m, void *data)
 		uint32_t off;
 
 		for (off = 0; off < ring->size; off += 4) {
-			uint32_t *ptr = (uint32_t *)(virt + off);
-			seq_printf(m, "%08x :  %08x\n", off, *ptr);
+			uint32_t __iomem *ptr = (uint32_t __iomem *)(virt + off);
+			seq_printf(m, "%08x :  %08x\n", off, ioread32(ptr));
 		}
 	}
 	mutex_unlock(&dev->struct_mutex);
@@ -1353,17 +1353,25 @@ static int i915_opregion(struct seq_file *m, void *unused)
 	struct drm_device *dev = node->minor->dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_opregion *opregion = &dev_priv->opregion;
+	void *data = kmalloc(OPREGION_SIZE, GFP_KERNEL);
 	int ret;
 
+	if (data == NULL)
+		return -ENOMEM;
+
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
 	if (ret)
-		return ret;
+		goto out;
 
-	if (opregion->header)
-		seq_write(m, opregion->header, OPREGION_SIZE);
+	if (opregion->header) {
+		memcpy_fromio(data, opregion->header, OPREGION_SIZE);
+		seq_write(m, data, OPREGION_SIZE);
+	}
 
 	mutex_unlock(&dev->struct_mutex);
 
+out:
+	kfree(data);
 	return 0;
 }
 
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/9] drm/i915: [sparse] __iomem fixes for ringbuffer
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
                   ` (4 preceding siblings ...)
  2012-04-16 21:07 ` [PATCH 5/9] drm/i915: [sparse] __iomem fixes for debugfs Ben Widawsky
@ 2012-04-16 21:07 ` Ben Widawsky
  2012-04-16 21:28   ` Chris Wilson
  2012-04-16 21:07 ` [PATCH 7/9] drm/i915: [sparse] forced __iomem ringbuffer fixes Ben Widawsky
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

Only one case of a "forced" casting where we use ring->map.handle and
cast it to __iomem. As with other patches, this should be safe always.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index caec699..62e11c5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -875,7 +875,7 @@ static int init_status_page(struct intel_ring_buffer *ring)
 		goto err_unpin;
 	}
 	ring->status_page.obj = obj;
-	memset(ring->status_page.page_addr, 0, PAGE_SIZE);
+	memset_io(ring->status_page.page_addr, 0, PAGE_SIZE);
 
 	intel_ring_setup_status_page(ring);
 	DRM_DEBUG_DRIVER("%s hws offset: 0x%08x\n",
@@ -938,7 +938,7 @@ static int intel_init_ring_buffer(struct drm_device *dev,
 		goto err_unpin;
 	}
 
-	ring->virtual_start = ring->map.handle;
+	ring->virtual_start = (void __iomem *)ring->map.handle;
 	ret = ring->init(ring);
 	if (ret)
 		goto err_unmap;
@@ -996,7 +996,7 @@ void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring)
 
 static int intel_wrap_ring_buffer(struct intel_ring_buffer *ring)
 {
-	unsigned int *virt;
+	void __iomem *virt;
 	int rem = ring->size - ring->tail;
 
 	if (ring->space < rem) {
@@ -1005,11 +1005,13 @@ static int intel_wrap_ring_buffer(struct intel_ring_buffer *ring)
 			return ret;
 	}
 
-	virt = (unsigned int *)(ring->virtual_start + ring->tail);
+	virt = ring->virtual_start + ring->tail;
 	rem /= 8;
 	while (rem--) {
-		*virt++ = MI_NOOP;
-		*virt++ = MI_NOOP;
+		iowrite32(MI_NOOP, virt);
+		virt+=4;
+		iowrite32(MI_NOOP, virt);
+		virt+=4;
 	}
 
 	ring->tail = 0;
@@ -1308,7 +1310,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 
 	if (!I915_NEED_GFX_HWS(dev)) {
 		ring->status_page.page_addr = dev_priv->status_page_dmah->vaddr;
-		memset(ring->status_page.page_addr, 0, PAGE_SIZE);
+		memset_io(ring->status_page.page_addr, 0, PAGE_SIZE);
 	}
 
 	return intel_init_ring_buffer(dev, ring);
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/9] drm/i915: [sparse] forced __iomem ringbuffer fixes
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
                   ` (5 preceding siblings ...)
  2012-04-16 21:07 ` [PATCH 6/9] drm/i915: [sparse] __iomem fixes for ringbuffer Ben Widawsky
@ 2012-04-16 21:07 ` Ben Widawsky
  2012-04-16 21:07 ` [PATCH 8/9] drm/i915: [sparse] __iomem fixes for gem Ben Widawsky
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

It should be safe to do this since GEN driver is only ever for x86, an
__iomem and dma mem should be the same. However, these changes caused me
to really think and try multiple solutions. Someone may come along and
decide to do something better with these.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 62e11c5..8a0050f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -869,7 +869,7 @@ static int init_status_page(struct intel_ring_buffer *ring)
 	}
 
 	ring->status_page.gfx_addr = obj->gtt_offset;
-	ring->status_page.page_addr = kmap(obj->pages[0]);
+	ring->status_page.page_addr = (uint32_t __iomem *)kmap(obj->pages[0]);
 	if (ring->status_page.page_addr == NULL) {
 		memset(&dev_priv->hws_map, 0, sizeof(dev_priv->hws_map));
 		goto err_unpin;
@@ -1309,7 +1309,8 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 
 
 	if (!I915_NEED_GFX_HWS(dev)) {
-		ring->status_page.page_addr = dev_priv->status_page_dmah->vaddr;
+		ring->status_page.page_addr =
+			(void __iomem *) dev_priv->status_page_dmah->vaddr;
 		memset_io(ring->status_page.page_addr, 0, PAGE_SIZE);
 	}
 
@@ -1350,7 +1351,8 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size)
 	ring->cleanup = render_ring_cleanup;
 
 	if (!I915_NEED_GFX_HWS(dev))
-		ring->status_page.page_addr = dev_priv->status_page_dmah->vaddr;
+		ring->status_page.page_addr =
+			(void __iomem *) dev_priv->status_page_dmah->vaddr;
 
 	ring->dev = dev;
 	INIT_LIST_HEAD(&ring->active_list);
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 8/9] drm/i915: [sparse] __iomem fixes for gem
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
                   ` (6 preceding siblings ...)
  2012-04-16 21:07 ` [PATCH 7/9] drm/i915: [sparse] forced __iomem ringbuffer fixes Ben Widawsky
@ 2012-04-16 21:07 ` Ben Widawsky
  2012-04-16 21:07 ` [PATCH 9/9] drm/i915: [sparse] __iomem fixes for intel_bios Ben Widawsky
  2012-04-21 20:59 ` [PATCH 0/9] i915 fixes for sparse warnings Daniel Vetter
  9 siblings, 0 replies; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

As with one of the earlier patches in the series, we're forced to cast
for copy_[to|from]_user. Again because of the nature of the GEN x86
exclusivity, this should be safe.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9415c07..96527db 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -266,8 +266,8 @@ __copy_to_user_swizzled(char __user *cpu_vaddr,
 }
 
 static inline int
-__copy_from_user_swizzled(char __user *gpu_vaddr, int gpu_offset,
-			  const char *cpu_vaddr,
+__copy_from_user_swizzled(char *gpu_vaddr, int gpu_offset,
+			  const char __user *cpu_vaddr,
 			  int length)
 {
 	int ret, cpu_offset = 0;
@@ -542,12 +542,13 @@ fast_user_write(struct io_mapping *mapping,
 		char __user *user_data,
 		int length)
 {
-	char *vaddr_atomic;
+	void __iomem *vaddr_atomic;
 	unsigned long unwritten;
 
 	vaddr_atomic = io_mapping_map_atomic_wc(mapping, page_base);
-	unwritten = __copy_from_user_inatomic_nocache(vaddr_atomic + page_offset,
-						      user_data, length);
+	unwritten = __copy_from_user_inatomic_nocache(
+				(void __force*)vaddr_atomic + page_offset,
+				user_data, length);
 	io_mapping_unmap_atomic(vaddr_atomic);
 	return unwritten;
 }
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 9/9] drm/i915: [sparse] __iomem fixes for intel_bios
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
                   ` (7 preceding siblings ...)
  2012-04-16 21:07 ` [PATCH 8/9] drm/i915: [sparse] __iomem fixes for gem Ben Widawsky
@ 2012-04-16 21:07 ` Ben Widawsky
  2012-04-21 20:59 ` [PATCH 0/9] i915 fixes for sparse warnings Daniel Vetter
  9 siblings, 0 replies; 16+ messages in thread
From: Ben Widawsky @ 2012-04-16 21:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

This is only the first part of the patch which does the correct types.
The fix for dereferencing stuff was taking too long, so I've stopped.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 3534593..7233e72 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -697,24 +697,27 @@ intel_parse_bios(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct pci_dev *pdev = dev->pdev;
-	struct bdb_header *bdb = NULL;
+	struct bdb_header __iomem *bdb = NULL;
 	u8 __iomem *bios = NULL;
 
 	init_vbt_defaults(dev_priv);
 
 	/* XXX Should this validation be moved to intel_opregion.c? */
 	if (!dmi_check_system(intel_no_opregion_vbt) && dev_priv->opregion.vbt) {
-		struct vbt_header *vbt = dev_priv->opregion.vbt;
-		if (memcmp(vbt->signature, "$VBT", 4) == 0) {
+		char sig[20];
+		struct vbt_header __iomem *vbt = dev_priv->opregion.vbt;
+		memcpy_fromio(sig, &vbt->signature, sizeof(sig));
+		if (memcmp(sig, "$VBT", 4) == 0) {
 			DRM_DEBUG_KMS("Using VBT from OpRegion: %20s\n",
 					 vbt->signature);
-			bdb = (struct bdb_header *)((char *)vbt + vbt->bdb_offset);
+			bdb = (struct bdb_header __iomem *)((char __iomem *)vbt +
+				ioread32(&vbt->bdb_offset));
 		} else
 			dev_priv->opregion.vbt = NULL;
 	}
 
 	if (bdb == NULL) {
-		struct vbt_header *vbt = NULL;
+		struct vbt_header __iomem *vbt = NULL;
 		size_t size;
 		int i;
 
@@ -724,8 +727,9 @@ intel_parse_bios(struct drm_device *dev)
 
 		/* Scour memory looking for the VBT signature */
 		for (i = 0; i + 4 < size; i++) {
-			if (!memcmp(bios + i, "$VBT", 4)) {
-				vbt = (struct vbt_header *)(bios + i);
+			u32 temp = ioread32(bios + i);
+			if (!memcmp(&temp, "$VBT", 4)) {
+				vbt = (struct vbt_header __iomem *)(bios + i);
 				break;
 			}
 		}
@@ -736,7 +740,7 @@ intel_parse_bios(struct drm_device *dev)
 			return -1;
 		}
 
-		bdb = (struct bdb_header *)(bios + i + vbt->bdb_offset);
+		bdb = (struct bdb_header __iomem *)(bios + i + vbt->bdb_offset);
 	}
 
 	/* Grab useful general definitions */
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/9] drm/i915: [sparse] trivial sparse fixes
  2012-04-16 21:07 ` [PATCH 1/9] drm/i915: [sparse] trivial sparse fixes Ben Widawsky
@ 2012-04-16 21:13   ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2012-04-16 21:13 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

On Mon, 16 Apr 2012 14:07:40 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> This should contain all the changes which require no thought to make
> sparse happy.

In a couple of spots, we make a structure/array static. Are they
suitable for const as well?

Otherwise, looks sane:
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/9] drm/i915: [sparse] __iomem fixes for ringbuffer
  2012-04-16 21:07 ` [PATCH 6/9] drm/i915: [sparse] __iomem fixes for ringbuffer Ben Widawsky
@ 2012-04-16 21:28   ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2012-04-16 21:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Ben Widawsky

On Mon, 16 Apr 2012 14:07:45 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
>  	rem /= 8;
>  	while (rem--) {
> -		*virt++ = MI_NOOP;
> -		*virt++ = MI_NOOP;
> +		iowrite32(MI_NOOP, virt);
> +		virt+=4;
> +		iowrite32(MI_NOOP, virt);
> +		virt+=4;
Might as well use memset_io().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/9] drm/i915: [sparse] don't use variable size arrays
  2012-04-16 21:07 ` [PATCH 2/9] drm/i915: [sparse] don't use variable size arrays Ben Widawsky
@ 2012-04-18  1:39   ` Paulo Zanoni
  2012-04-18  2:20     ` Ben Widawsky
  0 siblings, 1 reply; 16+ messages in thread
From: Paulo Zanoni @ 2012-04-18  1:39 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx, Ben Widawsky

2012/4/16 Ben Widawsky <ben@bwidawsk.net>:
> Sparse doesn't like:
> "error: bad constant expression"
>

<bikeshedding>
I know you'll hate me for asking, but: how difficult is it to fix sparse?
Adding those mallocs/frees increases the code complexity, making it
harder to read...
</bikeshedding>

-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/9] drm/i915: [sparse] don't use variable size arrays
  2012-04-18  1:39   ` Paulo Zanoni
@ 2012-04-18  2:20     ` Ben Widawsky
  2012-04-18  9:05       ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Ben Widawsky @ 2012-04-18  2:20 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Ben Widawsky

On Tue, 17 Apr 2012 22:39:15 -0300
Paulo Zanoni <przanoni@gmail.com> wrote:

> 2012/4/16 Ben Widawsky <ben@bwidawsk.net>:
> > Sparse doesn't like:
> > "error: bad constant expression"
> >
> 
> <bikeshedding>
> I know you'll hate me for asking, but: how difficult is it to fix sparse?
> Adding those mallocs/frees increases the code complexity, making it
> harder to read...
> </bikeshedding>
> 

I don't consider this a bikeshed. I've always been "under the
impression" C99 was sort of taboo in the kernel. In this case
specifically, it's never a great idea to allocate an unknown amount of
stack space as it probably messes with some of the static tools and
such.

In other words, I believe the right thing to do here is not to fix
sparse. Plus there is precedent in other drivers to fix this kind of
thing for sparse. I originally had this patch create an arbitrarily
large object on the stack and fail if the args_len was too big. I can
go back to that certainly if people prefer.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/9] drm/i915: [sparse] don't use variable size arrays
  2012-04-18  2:20     ` Ben Widawsky
@ 2012-04-18  9:05       ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2012-04-18  9:05 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx, Ben Widawsky

On Tue, Apr 17, 2012 at 07:20:35PM -0700, Ben Widawsky wrote:
> On Tue, 17 Apr 2012 22:39:15 -0300
> Paulo Zanoni <przanoni@gmail.com> wrote:
> 
> > 2012/4/16 Ben Widawsky <ben@bwidawsk.net>:
> > > Sparse doesn't like:
> > > "error: bad constant expression"
> > >
> > 
> > <bikeshedding>
> > I know you'll hate me for asking, but: how difficult is it to fix sparse?
> > Adding those mallocs/frees increases the code complexity, making it
> > harder to read...
> > </bikeshedding>
> > 
> 
> I don't consider this a bikeshed. I've always been "under the
> impression" C99 was sort of taboo in the kernel. In this case
> specifically, it's never a great idea to allocate an unknown amount of
> stack space as it probably messes with some of the static tools and
> such.
> 
> In other words, I believe the right thing to do here is not to fix
> sparse. Plus there is precedent in other drivers to fix this kind of
> thing for sparse. I originally had this patch create an arbitrarily
> large object on the stack and fail if the args_len was too big. I can
> go back to that certainly if people prefer.

I've picked up the first 2 patches of this series for -next, with a tiny
bikeshed on the 2nd one. I've gotten stuck while reviewing the next one,
I think it'd be good if we can quickly discuss these on irc.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/9] i915 fixes for sparse warnings.
  2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
                   ` (8 preceding siblings ...)
  2012-04-16 21:07 ` [PATCH 9/9] drm/i915: [sparse] __iomem fixes for intel_bios Ben Widawsky
@ 2012-04-21 20:59 ` Daniel Vetter
  9 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2012-04-21 20:59 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

On Mon, Apr 16, 2012 at 02:07:39PM -0700, Ben Widawsky wrote:
> Patch 1/2 is a mix of fairly trivial stuff. I see no reason to not take
> these.
> Patch 3-9 Are all __iomem related. These should run through some
> regression testing.
> 
> Most of the __iomem stuff fell into place fairly easily, but it's
> definitely subject to a lot of copy/paste errors, since trying to
> automate the replacement didn't workout well. Throughout the patches, I
> tried really hard to not force cast things, but a couple of situations
> cropped up which gave me no choice. What I hope to achieve by that is
> there should be no functional or performance changes (aside from
> copy/paste).
> 
> I've given up on fixing intel_bios.c completely.
> 
> These are based off of dinq as of yesterday.

Ok, after some lengthy irc bikeshedding with Ben and Chris I've picked up
a few more of these, but changed a few things:
- Any hunk that touched the status_page got dropped. I've looked around
  and I think I have a plan to fix that disaster for real.
- Added comments about pointer casts due to sparse.
- Dropped the intel_bios.c patch, we agreed that to be too horrible. This
  needs some serious care to be sparse-compliant.

Thanks for digging through this mess.
-Daniel

> 
> 
> Ben Widawsky (9):
>   drm/i915: [sparse] trivial sparse fixes
>   drm/i915: [sparse] don't use variable size arrays
>   drm/i915: [sparse] __iomem fixes for opregion
>   drm/i915: [sparse] __iomem fixes for overlay
>   drm/i915: [sparse] __iomem fixes for debugfs
>   drm/i915: [sparse] __iomem fixes for ringbuffer
>   drm/i915: [sparse] forced __iomem ringbuffer fixes
>   drm/i915: [sparse] __iomem fixes for gem
>   drm/i915: [sparse] __iomem fixes for intel_bios
> 
>  drivers/gpu/drm/i915/i915_debugfs.c      |   28 ++++---
>  drivers/gpu/drm/i915/i915_drv.h          |   12 +--
>  drivers/gpu/drm/i915/i915_gem.c          |   11 +--
>  drivers/gpu/drm/i915/i915_ioc32.c        |    5 +-
>  drivers/gpu/drm/i915/i915_trace_points.c |    2 +
>  drivers/gpu/drm/i915/intel_acpi.c        |    1 +
>  drivers/gpu/drm/i915/intel_bios.c        |   20 +++--
>  drivers/gpu/drm/i915/intel_display.c     |    2 +-
>  drivers/gpu/drm/i915/intel_fb.c          |    2 +-
>  drivers/gpu/drm/i915/intel_opregion.c    |   67 ++++++++-------
>  drivers/gpu/drm/i915/intel_overlay.c     |  130 +++++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_ringbuffer.c  |   28 ++++---
>  drivers/gpu/drm/i915/intel_sdvo.c        |   26 ++++--
>  13 files changed, 197 insertions(+), 137 deletions(-)
> 
> -- 
> 1.7.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-04-21 20:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-16 21:07 [PATCH 0/9] i915 fixes for sparse warnings Ben Widawsky
2012-04-16 21:07 ` [PATCH 1/9] drm/i915: [sparse] trivial sparse fixes Ben Widawsky
2012-04-16 21:13   ` Chris Wilson
2012-04-16 21:07 ` [PATCH 2/9] drm/i915: [sparse] don't use variable size arrays Ben Widawsky
2012-04-18  1:39   ` Paulo Zanoni
2012-04-18  2:20     ` Ben Widawsky
2012-04-18  9:05       ` Daniel Vetter
2012-04-16 21:07 ` [PATCH 3/9] drm/i915: [sparse] __iomem fixes for opregion Ben Widawsky
2012-04-16 21:07 ` [PATCH 4/9] drm/i915: [sparse] __iomem fixes for overlay Ben Widawsky
2012-04-16 21:07 ` [PATCH 5/9] drm/i915: [sparse] __iomem fixes for debugfs Ben Widawsky
2012-04-16 21:07 ` [PATCH 6/9] drm/i915: [sparse] __iomem fixes for ringbuffer Ben Widawsky
2012-04-16 21:28   ` Chris Wilson
2012-04-16 21:07 ` [PATCH 7/9] drm/i915: [sparse] forced __iomem ringbuffer fixes Ben Widawsky
2012-04-16 21:07 ` [PATCH 8/9] drm/i915: [sparse] __iomem fixes for gem Ben Widawsky
2012-04-16 21:07 ` [PATCH 9/9] drm/i915: [sparse] __iomem fixes for intel_bios Ben Widawsky
2012-04-21 20:59 ` [PATCH 0/9] i915 fixes for sparse warnings Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.