Skip to contentSkip to conversation

gma500: Only register interrupt handler for poulsbo hardware

PATCH In review Alan Cox 2 participants 12 messages Aug 26, 2011 Staging

lore
Share & tools
lore

Alan Cox

author
#0 lore raw

PATCH 1/9 gma500: Only register interrupt handler for poulsbo hardware

From: Patrik Jakobsson <[email protected]>

First step in adding proper irq handling. We'll start with poulsbo support so make sure other chips don't touch drm_irq_install().

---

 drivers/staging/gma500/psb_drv.c |    2 +-
 drivers/staging/gma500/psb_drv.h |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)
drivers/staging/gma500/psb_drv.c +1 -1
index b2cdce7..d91083a 100644
@@ -410,7 +410,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset)
 	PSB_WVDC32(0x00000000, PSB_INT_ENABLE_R);
 	PSB_WVDC32(0xFFFFFFFF, PSB_INT_MASK_R);
 	spin_unlock_irqrestore(&dev_priv->irqmask_lock, irqflags);
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
+	if (IS_PSB(dev) && drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_irq_install(dev);
 
 	dev->vblank_disable_allowed = 1;
drivers/staging/gma500/psb_drv.h +1 -0
index fd4732d..8cd4f1a 100644
@@ -43,6 +43,7 @@ enum {
 	CHIP_MFLD_0130 = 3,		/* Medfield */
 };
 
+#define IS_PSB(dev) (((dev)->pci_device & 0xfffe) == 0x8108)
 #define IS_MRST(dev) (((dev)->pci_device & 0xfffc) == 0x4100)
 #define IS_MFLD(dev) (((dev)->pci_device & 0xfff8) == 0x0130)
 
author
#1 lore raw

PATCH 2/9 gma500: Don't enable MSI on Poulsbo

From: Patrik Jakobsson <[email protected]>

Chipset reports MSI capabilities for Poulsbo even though it isn't really there.

---

 drivers/staging/gma500/psb_drv.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/staging/gma500/psb_drv.c +5 -3
index d91083a..10c8ecd 100644
@@ -329,6 +329,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset)
 	dev_priv->dev = dev;
 	dev->dev_private = (void *) dev_priv;
 
+	if (!IS_PSB(dev)) {
+		if (pci_enable_msi(dev->pdev))
+			dev_warn(dev->dev, "Enabling MSI failed!\n");
+	}
+
 	dev_priv->num_pipe = dev_priv->ops->pipes;
 
 	resource_start = pci_resource_start(dev->pdev, PSB_MMIO_RESOURCE);
@@ -1207,9 +1212,6 @@ static struct pci_driver psb_pci_driver = {
 
 static int psb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
-	/* MLD Added this from Inaky's patch */
-	if (pci_enable_msi(pdev))
-		dev_warn(&pdev->dev, "Enable MSI failed!\n");
 	return drm_get_pci_dev(pdev, ent, &driver);
 }
 
author
#2 lore raw

PATCH 3/9 gma500: Add VBLANK support for Poulsbo hardware

In reply to Alan Cox

From: Patrik Jakobsson <[email protected]>

---

 drivers/staging/gma500/psb_drv.h |    3 ++
 drivers/staging/gma500/psb_irq.c |   71 ++++++++++++++++++++++----------------
 2 files changed, 44 insertions(+), 30 deletions(-)
drivers/staging/gma500/psb_drv.h +3 -0
index 8cd4f1a..9cbb494 100644
@@ -135,6 +135,9 @@ enum {
 #define _PSB_IRQ_MSVDX_FLAG	  (1<<19)
 #define _LNC_IRQ_TOPAZ_FLAG	  (1<<20)
 
+#define _PSB_PIPE_EVENT_FLAG	(_PSB_VSYNC_PIPEA_FLAG | \
+				 _PSB_VSYNC_PIPEB_FLAG)
+
 /* This flag includes all the display IRQ bits excepts the vblank irqs. */
 #define _MDFLD_DISP_ALL_IRQ_FLAG (_MDFLD_PIPEC_EVENT_FLAG | \
 				  _MDFLD_PIPEB_EVENT_FLAG | \
drivers/staging/gma500/psb_irq.c +41 -30

138 lines not on this page · raw message

author
#3 lore raw

PATCH 4/9 gma500: do a pass over the FIXME tags

In reply to Alan Cox

From: Alan Cox <[email protected]>

---

 drivers/staging/gma500/cdv_device.c          |    1 -
 drivers/staging/gma500/framebuffer.c         |    7 ++++--
 drivers/staging/gma500/gem.c                 |    5 +---
 drivers/staging/gma500/gtt.c                 |   30 ++++++++++++++++----------
 drivers/staging/gma500/intel_opregion.c      |    1 +
 drivers/staging/gma500/mdfld_intel_display.c |   12 ++++++----
 drivers/staging/gma500/mrst_crtc.c           |   16 ++++----------
 drivers/staging/gma500/psb_drv.c             |    1 -
 drivers/staging/gma500/psb_intel_lvds.c      |    1 -
 drivers/staging/gma500/psb_lid.c             |    2 --
 10 files changed, 38 insertions(+), 38 deletions(-)
drivers/staging/gma500/cdv_device.c +0 -1
index 87614e0..8ec10ca 100644
@@ -30,7 +30,6 @@
 #define VGA_SR_INDEX		0x3c4
 #define VGA_SR_DATA		0x3c5
 
-/* FIXME: should check if we are the active VGA device ?? */
 static void cdv_disable_vga(struct drm_device *dev)
 {
 	u8 sr1;
drivers/staging/gma500/framebuffer.c +5 -2
index ebfde13..7748331 100644
@@ -733,9 +733,12 @@ static void psb_setup_outputs(struct drm_device *dev)
 			clone_mask = (1 << INTEL_OUTPUT_MIPI2);
 			break;
 		case INTEL_OUTPUT_HDMI:
-			if (IS_MFLD(dev))
+		        /* HDMI on crtc 1 for SoC devices and crtc 0 for
+                           Cedarview. HDMI on Poulsbo is only via external
+			   logic */
+			if (IS_MFLD(dev) || IS_MRST(dev))
 				crtc_mask = (1 << 1);
-			else	/* FIXME: review Oaktrail */
+			else
 				crtc_mask = (1 << 0);	/* Cedarview */
 			clone_mask = (1 << INTEL_OUTPUT_HDMI);
 			break;
drivers/staging/gma500/gem.c +1 -4
index 65fdd6b..f6433c0 100644
@@ -120,8 +120,7 @@ static int psb_gem_create(struct drm_file *file,
 	/* Initialize the extra goodies GEM needs to do all the hard work */
 	if (drm_gem_object_init(dev, &r->gem, size) != 0) {
 		psb_gtt_free_range(dev, r);
-		/* GEM doesn't give an error code and we don't have an
-		   EGEMSUCKS so make something up for now - FIXME */
+		/* GEM doesn't give an error code so use -ENOMEM */
 		dev_err(dev->dev, "GEM init failed for %lld\n", size);
 		return -ENOMEM;
 	}
@@ -191,8 +190,6 @@ int psb_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev,
  *	The VMA was set up by GEM. In doing so it also ensured that the
  *	vma->vm_private_data points to the GEM object that is backing this
  *	mapping.
- *
- *	FIXME
  */
 int psb_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
drivers/staging/gma500/gtt.c +19 -11
index 461ead2..a246239 100644
@@ -72,9 +72,8 @@ u32 *psb_gtt_entry(struct drm_device *dev, struct gtt_range *r)
  *	@r: our GTT range
  *
  *	Take our preallocated GTT range and insert the GEM object into
- *	the GTT.
- *
- *	FIXME: gtt lock ?
+ *	the GTT. This is protected via the gtt mutex which the caller
+ *	must hold.
  */
 static int psb_gtt_insert(struct drm_device *dev, struct gtt_range *r)
 {
@@ -111,7 +110,8 @@ static int psb_gtt_insert(struct drm_device *dev, struct gtt_range *r)
  *	@r: our GTT range
  *
  *	Remove a preallocated GTT range from the GTT. Overwrite all the
- *	page table entries with the dummy page
+ *	page table entries with the dummy page. This is protected via the gtt
+ *	mutex which the caller must hold.
  */
 
 static void psb_gtt_remove(struct drm_device *dev, struct gtt_range *r)
@@ -136,7 +136,8 @@ static void psb_gtt_remove(struct drm_device *dev, struct gtt_range *r)
  *	@gt: the gtt range
  *
  *	Pin and build an in kernel list of the pages that back our GEM object.
- *	While we hold this the pages cannot be swapped out
+ *	While we hold this the pages cannot be swapped out. This is protected
+ *	via the gtt mutex which the caller must hold.
  */
 static int psb_gtt_attach_pages(struct gtt_range *gt)
 {
@@ -158,7 +159,7 @@ static int psb_gtt_attach_pages(struct gtt_range *gt)
 	gt->npage = pages;
 
 	for (i = 0; i < pages; i++) {
-		/* FIXME: review flags later */
+		/* FIXME: needs updating as per mail from Hugh Dickins */
 		p = read_cache_page_gfp(mapping, i,
 					__GFP_COLD | GFP_KERNEL);
 		if (IS_ERR(p))
@@ -181,7 +182,8 @@ err:
  *
  *	Undo the effect of psb_gtt_attach_pages. At this point the pages
  *	must have been removed from the GTT as they could now be paged out
- *	and move bus address.
+ *	and move bus address. This is protected via the gtt mutex which the
+ *	caller must hold.
  */
 static void psb_gtt_detach_pages(struct gtt_range *gt)
 {
@@ -390,15 +392,18 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 	pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
 
 	/*
-	 *	FIXME: video mmu has hw bug to access 0x0D0000000,
-	 *	then make gatt start at 0x0e000,0000
+	 *	The video mmu has a hw bug when accessing 0x0D0000000.
+	 *	Make gatt start at 0x0e000,0000. This doesn't actually
+	 *	matter for us but may do if the video acceleration ever
+	 *	gets opened up.
 	 */
 	pg->mmu_gatt_start = 0xE0000000;
 
 	pg->gtt_start = pci_resource_start(dev->pdev, PSB_GTT_RESOURCE);
 	gtt_pages = pci_resource_len(dev->pdev, PSB_GTT_RESOURCE)
 								>> PAGE_SHIFT;
-	/* CDV workaround */
+	/* Some CDV firmware doesn't report this currently. In which case the
+	   system has 64 gtt pages */
 	if (pg->gtt_start == 0 || gtt_pages == 0) {
 		dev_err(dev->dev, "GTT PCI BAR not initialized.\n");
 		gtt_pages = 64;
@@ -412,13 +417,16 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 
 	if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
 		static struct resource fudge;	/* Preferably peppermint */
-
 		/* This can occur on CDV SDV systems. Fudge it in this case.
 		   We really don't care what imaginary space is being allocated
 		   at this point */
 		dev_err(dev->dev, "GATT PCI BAR not initialized.\n");
 		pg->gatt_start = 0x40000000;
 		pg->gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
+		/* This is a little confusing but in fact the GTT is providing
+		   a view from the GPU into memory and not vice versa. As such
+		   this is really allocating space that is not the same as the
+		   CPU address space on CDV */
 		fudge.start = 0x40000000;
 		fudge.end = 0x40000000 + 128 * 1024 * 1024 - 1;
 		fudge.name = "fudge";
drivers/staging/gma500/intel_opregion.c +1 -0
index d2e6037..d946bc1 100644
@@ -20,6 +20,7 @@
  * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
  * DEALINGS IN THE SOFTWARE.
  *
+ * FIXME: resolve with the i915 version
  */
 
 #include "psb_drv.h"
drivers/staging/gma500/mdfld_intel_display.c +7 -5
index aa2ff55..8eb827e 100644
@@ -282,7 +282,7 @@ static int mdfld_intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 		return -EINVAL;
 	}
 		
-#if 1 /* FIXME_JLIU7 can't enalbe cursorB/C HW issue. need to remove after HW fix */
+#if 1 /* FIXME_JLIU7 can't enable cursorB/C HW issue. need to remove after HW fix */
 	if (pipe != 0)
 		return 0;
 #endif 
@@ -484,7 +484,7 @@ void mdfld_disable_crtc (struct drm_device *dev, int pipe)
 
 	/* FIXME_JLIU7 MDFLD_PO revisit */
 	/* Wait for vblank for the disable to take effect */
-// MDFLD_PO_JLIU7		psb_intel_wait_for_vblank(dev);
+/* MDFLD_PO_JLIU7		psb_intel_wait_for_vblank(dev); */
 
 	/* Next, disable display pipes */
 	temp = REG_READ(pipeconf_reg);
@@ -561,7 +561,6 @@ static void mdfld_crtc_dpms(struct drm_crtc *crtc, int mode)
 	    //gbdispstatus = true;
 	}
 
-
 /* FIXME_JLIU7 MDFLD_PO replaced w/ the following function */
 /* mdfld_dbi_dpms (struct drm_device *dev, int pipe, bool enabled) */
 
@@ -1150,8 +1149,11 @@ static int mdfld_crtc_mode_set(struct drm_crtc *crtc,
 			dev->mode_config.scaling_mode_property, &scalingType);
 
 	if (scalingType == DRM_MODE_SCALE_NO_SCALE) {
-		/*Moorestown doesn't have register support for centering so we need to
-		  mess with the h/vblank and h/vsync start and ends to get centering*/
+		/*
+		 *	Medfield doesn't have register support for centering so
+		 *	we need to mess with the h/vblank and h/vsync start and
+		 *	ends to get central
+		 */
 		int offsetX = 0, offsetY = 0;
 
 		offsetX = (adjusted_mode->crtc_hdisplay - mode->crtc_hdisplay) / 2;
drivers/staging/gma500/mrst_crtc.c +5 -11
index 72464dd..c9311a5 100644
@@ -521,12 +521,11 @@ int mrst_pipe_set_base(struct drm_crtc *crtc,
 			    int x, int y, struct drm_framebuffer *old_fb)
 {
 	struct drm_device *dev = crtc->dev;
-	/* struct drm_i915_master_private *master_priv; */
 	struct psb_intel_crtc *psb_intel_crtc = to_psb_intel_crtc(crtc);
 	struct psb_framebuffer *psbfb = to_psb_fb(crtc->fb);
 	int pipe = psb_intel_crtc->pipe;
 	unsigned long start, offset;
-	/* FIXME: check if we need this surely MRST is pipe 0 only */
+
 	int dspbase = (pipe == 0 ? DSPALINOFF : DSPBBASE);
 	int dspsurf = (pipe == 0 ? DSPASURF : DSPBSURF);
 	int dspstride = (pipe == 0) ? DSPASTRIDE : DSPBSTRIDE;
@@ -572,15 +571,10 @@ int mrst_pipe_set_base(struct drm_crtc *crtc,
 	}
 	REG_WRITE(dspcntr_reg, dspcntr);
 
-	if (0 /* FIXMEAC - check what PSB needs */) {
-		REG_WRITE(dspbase, offset);
-		REG_READ(dspbase);
-		REG_WRITE(dspsurf, start);
-		REG_READ(dspsurf);
-	} else {
-		REG_WRITE(dspbase, start + offset);
-		REG_READ(dspbase);
-	}
+	REG_WRITE(dspbase, offset);
+	REG_READ(dspbase);
+	REG_WRITE(dspsurf, start);
+	REG_READ(dspsurf);
 
 pipe_set_base_exit:
 	gma_power_end(dev);
drivers/staging/gma500/psb_drv.c +0 -1
index 10c8ecd..34cf24a 100644
@@ -183,7 +183,6 @@ static void psb_lastclose(struct drm_device *dev)
 
 static void psb_do_takedown(struct drm_device *dev)
 {
-	/* FIXME: do we need to clean up the gtt here ? */
 }
 
 static int psb_do_init(struct drm_device *dev)
drivers/staging/gma500/psb_intel_lvds.c +0 -1
index c6436da..8b951e3 100644
@@ -382,7 +382,6 @@ bool psb_intel_lvds_mode_fixup(struct drm_encoder *encoder,
 	if (psb_intel_output->type == INTEL_OUTPUT_MIPI2)
 		panel_fixed_mode = mode_dev->panel_fixed_mode2;
 
-	/* FIXME: review for Medfield */
 	/* PSB requires the LVDS is on pipe B, MRST has only one pipe anyway */
 	if (!IS_MRST(dev) && psb_intel_crtc->pipe == 0) {
 		printk(KERN_ERR "Can't support LVDS on pipe A\n");
drivers/staging/gma500/psb_lid.c +0 -2
index af32851..b867aab 100644
@@ -52,8 +52,6 @@ static void psb_lid_timer_func(unsigned long data)
 			pp_status = REG_READ(PP_STATUS);
 		} while ((pp_status & PP_ON) == 0);
 	}
-		/* printk(KERN_INFO"%s: lid: closed\n", __FUNCTION__); */
-
 	dev_priv->lid_last_state =  readl(lid_state);
 
 lid_timer_schedule:
author
#4 lore raw

PATCH 5/9 gma500: Convert spaces to tabs in accel_2d.c.

In reply to Alan Cox

From: Akshay Joshi <[email protected]>

Convert the spaces within the accel_2d.c file to tabs in order to comply with the coding style of the kernel.

---

 drivers/staging/gma500/accel_2d.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
drivers/staging/gma500/accel_2d.c +7 -7
index 14400fc..be92371 100644
@@ -118,7 +118,7 @@ static int psbfb_2d_submit(struct drm_psb_private *dev_priv, uint32_t *cmdbuf,
 		size -= submit_size;
 		ret = psb_2d_wait_available(dev_priv, submit_size);
 		if (ret)
-		        break;
+			break;
 
 		submit_size <<= 2;
 
@@ -388,19 +388,19 @@ int psb_accel_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	for (i = 0; i < op->size; i++, op_ptr++) {
 		u32 r = *op_ptr & 0xF0000000;
 		/* Fill in the GTT offsets for the command buffer */
-        	if (r == PSB_2D_SRC_SURF_BH ||
-			r == PSB_2D_DST_SURF_BH || 
+		if (r == PSB_2D_SRC_SURF_BH ||
+			r == PSB_2D_DST_SURF_BH ||
 			r == PSB_2D_MASK_SURF_BH ||
 			r == PSB_2D_PAT_SURF_BH) {
 			i++;
 			op_ptr++;
 			if (i == op->size)
 				goto bad;
-                        if (*op_ptr)
+			if (*op_ptr)
 				goto bad;
-                        *op_ptr = gtt->offset;
-                        continue;
-                }
+			*op_ptr = gtt->offset;
+			continue;
+		}
 	}
 	psbfb_2d_submit(dev_priv, op->cmd, op->size);
 	err = 0;
author
#5 lore raw

PATCH 6/9 gma500: kill bogus code

In reply to Alan Cox

From: Alan Cox <[email protected]>

During the power split ups and work a chunk of code escaped into the Poulsbo code path which it isn't for. On some devices such as the Dell mini-10 this causes problems.

---

 drivers/staging/gma500/psb_device.c |   32 --------------------------------
 1 files changed, 0 insertions(+), 32 deletions(-)
drivers/staging/gma500/psb_device.c +0 -32
index 4659132..b97aa78 100644
@@ -213,7 +213,6 @@ static int psb_restore_display_registers(struct drm_device *dev)
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
 	struct drm_connector *connector;
-	int pp_stat;
 
 	/* Display arbitration + watermarks */
 	PSB_WVDC32(dev_priv->saveDSPARB, DSPARB);
@@ -237,37 +236,6 @@ static int psb_restore_display_registers(struct drm_device *dev)
 		connector->funcs->restore(connector);
 
 	mutex_unlock(&dev->mode_config.mutex);
-
-	if (dev_priv->iLVDS_enable) {
-		/*shutdown the panel*/
-		PSB_WVDC32(0, PP_CONTROL);
-		do {
-			pp_stat = PSB_RVDC32(PP_STATUS);
-		} while (pp_stat & 0x80000000);
-
-		/* Turn off the plane */
-		PSB_WVDC32(0x58000000, DSPACNTR);
-		PSB_WVDC32(0, DSPASURF);/*trigger the plane disable*/
-		/* Wait ~4 ticks */
-		msleep(4);
-		/* Turn off pipe */
-		PSB_WVDC32(0x0, PIPEACONF);
-		/* Wait ~8 ticks */
-		msleep(8);
-
-		/* Turn off PLLs */
-		PSB_WVDC32(0, MRST_DPLL_A);
-	} else {
-		PSB_WVDC32(DPI_SHUT_DOWN, DPI_CONTROL_REG);
-		PSB_WVDC32(0x0, PIPEACONF);
-		PSB_WVDC32(0x2faf0000, BLC_PWM_CTL);
-		while (REG_READ(0x70008) & 0x40000000)
-			cpu_relax();
-		while ((PSB_RVDC32(GEN_FIFO_STAT_REG) & DPI_FIFO_EMPTY)
-			!= DPI_FIFO_EMPTY)
-			cpu_relax();
-		PSB_WVDC32(0, DEVICE_READY_REG);
-	}
 	return 0;
 }
 
author
#6 lore raw

PATCH 7/9 gma500: Fix backlight crash

In reply to Alan Cox

From: Alan Cox <[email protected]>

Initial changes to get backlight behaviour we want and to fix backlight crashes on suspend/resume paths.

[Note: on some boxes this will now produce a warning about the backlight, this
isn't a regression it's an unfixed but non harmful case I still need to nail]

---

 drivers/staging/gma500/power.c          |    2 +
 drivers/staging/gma500/psb_intel_lvds.c |   60 +++++++++++++++++--------------
 2 files changed, 34 insertions(+), 28 deletions(-)
drivers/staging/gma500/power.c +1 -1
index 972bea7..9402569 100644
@@ -302,7 +302,7 @@ int psb_runtime_suspend(struct device *dev)
 
 int psb_runtime_resume(struct device *dev)
 {
-	return 0;
+	return gma_power_resume(dev);;
 }
 
 int psb_runtime_idle(struct device *dev)
drivers/staging/gma500/psb_intel_lvds.c +33 -27

132 lines not on this page · raw message

author
#7 lore raw

PATCH 8/9 gma500: frame buffer locking

In reply to Alan Cox

From: Alan Cox <[email protected]>

If we are the console then a printk can hit us with a spin lock held (and in fact the kernel will do its best to take printing lock).

In that case we cannot politely sleep when synching after an accelerated op but must behave obnixously to be sure of getting the bits out.

---

 drivers/staging/gma500/accel_2d.c |   10 ++++++----
 drivers/staging/gma500/psb_drv.c  |    2 +-
 drivers/staging/gma500/psb_drv.h  |    2 +-
 kernel/printk.c                   |    2 +-
 4 files changed, 9 insertions(+), 7 deletions(-)
drivers/staging/gma500/accel_2d.c +6 -4
index be92371..114b99a 100644
@@ -111,8 +111,9 @@ static int psbfb_2d_submit(struct drm_psb_private *dev_priv, uint32_t *cmdbuf,
 	int ret = 0;
 	int i;
 	unsigned submit_size;
+	unsigned long flags;
 
-	mutex_lock(&dev_priv->mutex_2d);
+	spin_lock_irqsave(&dev_priv->lock_2d, flags);
 	while (size > 0) {
 		submit_size = (size < 0x60) ? size : 0x60;
 		size -= submit_size;
@@ -127,7 +128,7 @@ static int psbfb_2d_submit(struct drm_psb_private *dev_priv, uint32_t *cmdbuf,
 
 		(void)PSB_RSGX32(PSB_SGX_2D_SLAVE_PORT + i - 4);
 	}
-	mutex_unlock(&dev_priv->mutex_2d);
+	spin_unlock_irqrestore(&dev_priv->lock_2d, flags);
 	return ret;
 }
 
@@ -327,8 +328,9 @@ int psbfb_sync(struct fb_info *info)
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	unsigned long _end = jiffies + DRM_HZ;
 	int busy = 0;
+	unsigned long flags;
 
-	mutex_lock(&dev_priv->mutex_2d);
+	spin_lock_irqsave(&dev_priv->lock_2d, flags);
 	/*
 	 * First idle the 2D engine.
 	 */
@@ -357,7 +359,7 @@ int psbfb_sync(struct fb_info *info)
 					_PSB_C2B_STATUS_BUSY) != 0);
 
 out:
-	mutex_unlock(&dev_priv->mutex_2d);
+	spin_unlock_irqrestore(&dev_priv->lock_2d, flags);
 	return (busy) ? -EBUSY : 0;
 }
 
drivers/staging/gma500/psb_drv.c +1 -1
index 34cf24a..9470af9 100644
@@ -228,7 +228,7 @@ static int psb_do_init(struct drm_device *dev)
 
 
 	spin_lock_init(&dev_priv->irqmask_lock);
-	mutex_init(&dev_priv->mutex_2d);
+	spin_lock_init(&dev_priv->lock_2d);
 
 	PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK0);
 	PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK1);
drivers/staging/gma500/psb_drv.h +1 -1
index 9cbb494..11d963a 100644
@@ -612,7 +612,7 @@ struct drm_psb_private {
 	void (*exit_idle)(struct drm_device *dev, u32 update_src);
 
 	/* 2D acceleration */
-	struct mutex mutex_2d;
+	spinlock_t lock_2d;
 
 	/* FIXME: Arrays anyone ? */
 	struct mdfld_dsi_encoder *encoder0;	
kernel/printk.c +1 -1
index e861488..af2eec8 100644
@@ -512,7 +512,7 @@ static void __call_console_drivers(unsigned start, unsigned end)
 	struct console *con;
 
 	for_each_console(con) {
-		if (exclusive_console && con != exclusive_console)
+		if (0 && exclusive_console && con != exclusive_console)
 			continue;
 		if ((con->flags & CON_ENABLED) && con->write &&
 				(cpu_online(smp_processor_id()) ||
#8 lore raw

Re: [PATCH 8/9] gma500: frame buffer locking

Quoted 92 lines · Alan Cox

From: Alan Cox <[email protected]>

If we are the console then a printk can hit us with a spin lock held (and in fact the kernel will do its best to take printing lock).

In that case we cannot politely sleep when synching after an accelerated op but must behave obnixously to be sure of getting the bits out.

---

 drivers/staging/gma500/accel_2d.c |   10 ++++++----
 drivers/staging/gma500/psb_drv.c  |    2 +-
 drivers/staging/gma500/psb_drv.h  |    2 +-
 kernel/printk.c                   |    2 +-
 4 files changed, 9 insertions(+), 7 deletions(-)
drivers/staging/gma500/accel_2d.c +6 -4
index be92371..114b99a 100644
@@ -111,8 +111,9 @@ static int psbfb_2d_submit(struct drm_psb_private *dev_priv, uint32_t *cmdbuf,
 	int ret = 0;
 	int i;
 	unsigned submit_size;
+	unsigned long flags;
 
-	mutex_lock(&dev_priv->mutex_2d);
+	spin_lock_irqsave(&dev_priv->lock_2d, flags);
 	while (size > 0) {
 		submit_size = (size < 0x60) ? size : 0x60;
 		size -= submit_size;
@@ -127,7 +128,7 @@ static int psbfb_2d_submit(struct drm_psb_private *dev_priv, uint32_t *cmdbuf,
 
 		(void)PSB_RSGX32(PSB_SGX_2D_SLAVE_PORT + i - 4);
 	}
-	mutex_unlock(&dev_priv->mutex_2d);
+	spin_unlock_irqrestore(&dev_priv->lock_2d, flags);
 	return ret;
 }
 
@@ -327,8 +328,9 @@ int psbfb_sync(struct fb_info *info)
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	unsigned long _end = jiffies + DRM_HZ;
 	int busy = 0;
+	unsigned long flags;
 
-	mutex_lock(&dev_priv->mutex_2d);
+	spin_lock_irqsave(&dev_priv->lock_2d, flags);
 	/*
 	 * First idle the 2D engine.
 	 */
@@ -357,7 +359,7 @@ int psbfb_sync(struct fb_info *info)
 					_PSB_C2B_STATUS_BUSY) != 0);
 
 out:
-	mutex_unlock(&dev_priv->mutex_2d);
+	spin_unlock_irqrestore(&dev_priv->lock_2d, flags);
 	return (busy) ? -EBUSY : 0;
 }
 
drivers/staging/gma500/psb_drv.c +1 -1
index 34cf24a..9470af9 100644
@@ -228,7 +228,7 @@ static int psb_do_init(struct drm_device *dev)
 
 
 	spin_lock_init(&dev_priv->irqmask_lock);
-	mutex_init(&dev_priv->mutex_2d);
+	spin_lock_init(&dev_priv->lock_2d);
 
 	PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK0);
 	PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK1);
drivers/staging/gma500/psb_drv.h +1 -1
index 9cbb494..11d963a 100644
@@ -612,7 +612,7 @@ struct drm_psb_private {
 	void (*exit_idle)(struct drm_device *dev, u32 update_src);
 
 	/* 2D acceleration */
-	struct mutex mutex_2d;
+	spinlock_t lock_2d;
 
 	/* FIXME: Arrays anyone ? */
 	struct mdfld_dsi_encoder *encoder0;	
kernel/printk.c +0 -0
index e861488..af2eec8 100644

kernel/printk.c static void __call_console_drivers(unsigned start, unsigned end) · line 518

Quoted 8 lines
kernel/printk.c +1 -1
@@ -512,7 +512,7 @@ static void __call_console_drivers(unsigned start, unsigned end)
 	struct console *con;
 
 	for_each_console(con) {
-		if (exclusive_console && con != exclusive_console)
+		if (0 && exclusive_console && con != exclusive_console)
 			continue;
 		if ((con->flags & CON_ENABLED) && con->write &&
 				(cpu_online(smp_processor_id()) ||

Are you sure you want this in the tree? I don't feel comfortable making this fix just for one staging driver right now, do you?

greg k-h

#9 lore raw

Re: [PATCH 8/9] gma500: frame buffer locking

Quoted 95 lines · Greg KH
Quoted 94 lines · Alan Cox

From: Alan Cox <[email protected]>

If we are the console then a printk can hit us with a spin lock held (and in fact the kernel will do its best to take printing lock).

In that case we cannot politely sleep when synching after an accelerated op but must behave obnixously to be sure of getting the bits out.

---

 drivers/staging/gma500/accel_2d.c |   10 ++++++----
 drivers/staging/gma500/psb_drv.c  |    2 +-
 drivers/staging/gma500/psb_drv.h  |    2 +-
 kernel/printk.c                   |    2 +-
 4 files changed, 9 insertions(+), 7 deletions(-)
drivers/staging/gma500/accel_2d.c +6 -4
index be92371..114b99a 100644
@@ -111,8 +111,9 @@ static int psbfb_2d_submit(struct drm_psb_private *dev_priv, uint32_t *cmdbuf,
 	int ret = 0;
 	int i;
 	unsigned submit_size;
+	unsigned long flags;
 
-	mutex_lock(&dev_priv->mutex_2d);
+	spin_lock_irqsave(&dev_priv->lock_2d, flags);
 	while (size > 0) {
 		submit_size = (size < 0x60) ? size : 0x60;
 		size -= submit_size;
@@ -127,7 +128,7 @@ static int psbfb_2d_submit(struct drm_psb_private *dev_priv, uint32_t *cmdbuf,
 
 		(void)PSB_RSGX32(PSB_SGX_2D_SLAVE_PORT + i - 4);
 	}
-	mutex_unlock(&dev_priv->mutex_2d);
+	spin_unlock_irqrestore(&dev_priv->lock_2d, flags);
 	return ret;
 }
 
@@ -327,8 +328,9 @@ int psbfb_sync(struct fb_info *info)
 	struct drm_psb_private *dev_priv = dev->dev_private;
 	unsigned long _end = jiffies + DRM_HZ;
 	int busy = 0;
+	unsigned long flags;
 
-	mutex_lock(&dev_priv->mutex_2d);
+	spin_lock_irqsave(&dev_priv->lock_2d, flags);
 	/*
 	 * First idle the 2D engine.
 	 */
@@ -357,7 +359,7 @@ int psbfb_sync(struct fb_info *info)
 					_PSB_C2B_STATUS_BUSY) != 0);
 
 out:
-	mutex_unlock(&dev_priv->mutex_2d);
+	spin_unlock_irqrestore(&dev_priv->lock_2d, flags);
 	return (busy) ? -EBUSY : 0;
 }
 
drivers/staging/gma500/psb_drv.c +1 -1
index 34cf24a..9470af9 100644
@@ -228,7 +228,7 @@ static int psb_do_init(struct drm_device *dev)
 
 
 	spin_lock_init(&dev_priv->irqmask_lock);
-	mutex_init(&dev_priv->mutex_2d);
+	spin_lock_init(&dev_priv->lock_2d);
 
 	PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK0);
 	PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK1);
drivers/staging/gma500/psb_drv.h +1 -1
index 9cbb494..11d963a 100644
@@ -612,7 +612,7 @@ struct drm_psb_private {
 	void (*exit_idle)(struct drm_device *dev, u32 update_src);
 
 	/* 2D acceleration */
-	struct mutex mutex_2d;
+	spinlock_t lock_2d;
 
 	/* FIXME: Arrays anyone ? */
 	struct mdfld_dsi_encoder *encoder0;	
kernel/printk.c +0 -0
index e861488..af2eec8 100644
@@ -512,7 +512,7 @@ static void __call_console_drivers(unsigned start, unsigned end)
 	struct console *con;
 
Quoted 9 lines

kernel/printk.c static void __call_console_drivers(unsigned start, unsigned end) · line 518

Quoted 6 lines
kernel/printk.c +1 -1
 	for_each_console(con) {
-		if (exclusive_console && con != exclusive_console)
+		if (0 && exclusive_console && con != exclusive_console)
 			continue;
 		if ((con->flags & CON_ENABLED) && con->write &&
 				(cpu_online(smp_processor_id()) ||

Are you sure you want this in the tree? I don't feel comfortable making this fix just for one staging driver right now, do you?

I've applied the other 8 patches here, skipping this one.

greg k-h

author
#10 lore raw

Re: [PATCH 8/9] gma500: frame buffer locking

In reply to Greg KH
Quoted 6 lines
Quoted 6 lines

index e861488..af2eec8 100644

kernel/printk.c +0 -0
@@ -512,7 +512,7 @@ static void __call_console_drivers(unsigned start, unsigned end)
 	struct console *con;
 
Quoted 9 lines

kernel/printk.c static void __call_console_drivers(unsigned start, unsigned end) · line 518

Quoted 6 lines
kernel/printk.c +1 -1
 	for_each_console(con) {
-		if (exclusive_console && con != exclusive_console)
+		if (0 && exclusive_console && con != exclusive_console)
 			continue;
 		if ((con->flags & CON_ENABLED) && con->write &&
 				(cpu_online(smp_processor_id()) ||

Are you sure you want this in the tree? I don't feel comfortable making this fix just for one staging driver right now, do you?

Oops no thats a debugging hack while using netconsole to chase out the bug.. that bit shouldn't be there !

author
#11 lore raw

PATCH 9/9 gma500: Fix up suspend/resume

In reply to Alan Cox

From: Alan Cox <[email protected]>

This isn't ideal as we could do with deferring the power on a lot more on Oaktrail and Medfield. We can't however do that without fixing other things first.

---

 drivers/staging/gma500/power.c             |    6 +++--
 drivers/staging/gma500/psb_drv.c           |   36 ++++++++++++----------------
 drivers/staging/gma500/psb_intel_display.c |    4 ---
 3 files changed, 20 insertions(+), 26 deletions(-)
drivers/staging/gma500/power.c +4 -2
index 9402569..436fe97 100644
@@ -83,7 +83,7 @@ static void gma_suspend_display(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->suspended)
+	if (!dev_priv->display_power)
 		return;
 	dev_priv->ops->save_regs(dev);
 	dev_priv->ops->power_down(dev);
@@ -101,7 +101,7 @@ static void gma_resume_display(struct pci_dev *pdev)
 	struct drm_device *dev = pci_get_drvdata(pdev);
 	struct drm_psb_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->suspended == false)
+	if (dev_priv->display_power)
 		return;
 
 	/* turn on the display power island */
@@ -265,6 +265,8 @@ bool gma_power_begin(struct drm_device *dev, bool force_on)
 	/* Ok power up needed */
 	ret = gma_resume_pci(dev->pdev);
 	if (ret == 0) {
+		/* FIXME: we want to defer this for Medfield/Oaktrail */
+		gma_resume_display(dev);
 		psb_irq_preinstall(dev);
 		psb_irq_postinstall(dev);
 		pm_runtime_get(&dev->pdev->dev);
drivers/staging/gma500/psb_drv.c +16 -20
index 9470af9..dc676c2 100644
@@ -448,12 +448,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset)
 
 	if (ret)
 		return ret;
-#if 0
-	/*enable runtime pm at last*/
-	pm_runtime_enable(&dev->pdev->dev);
+
+	/* Enable runtime pm at last */
 	pm_runtime_set_active(&dev->pdev->dev);
-#endif
-	/*Intel drm driver load is done, continue doing pvr load*/
 	return 0;
 out_err:
 	psb_driver_unload(dev);
@@ -470,14 +467,13 @@ static int psb_sizes_ioctl(struct drm_device *dev, void *data,
 			   struct drm_file *file_priv)
 {
 	struct drm_psb_private *dev_priv = psb_priv(dev);
-	struct drm_psb_sizes_arg *arg =
-		(struct drm_psb_sizes_arg *) data;
+	struct drm_psb_sizes_arg *arg = data;
 
 	*arg = dev_priv->sizes;
 	return 0;
 }
 
-static int psb_dc_state_ioctl(struct drm_device *dev, void * data,
+static int psb_dc_state_ioctl(struct drm_device *dev, void *data,
 				struct drm_file *file_priv)
 {
 	uint32_t flags;
@@ -485,8 +481,7 @@ static int psb_dc_state_ioctl(struct drm_device *dev, void * data,
 	struct drm_mode_object *obj;
 	struct drm_connector *connector;
 	struct drm_crtc *crtc;
-	struct drm_psb_dc_state_arg *arg =
-		(struct drm_psb_dc_state_arg *)data;
+	struct drm_psb_dc_state_arg *arg = data;
 
 
 	/* Double check MRST case */
@@ -1114,15 +1109,12 @@ static long psb_unlocked_ioctl(struct file *filp, unsigned int cmd,
 {
 	struct drm_file *file_priv = filp->private_data;
 	struct drm_device *dev = file_priv->minor->dev;
-	struct drm_psb_private *dev_priv = dev->dev_private;
-	static unsigned int runtime_allowed;
-
-	if (runtime_allowed == 1 && dev_priv->is_lvds_on) {
-		runtime_allowed++;
-		pm_runtime_allow(&dev->pdev->dev);
-		dev_priv->rpm_enabled = 1;
-	}
-	return drm_ioctl(filp, cmd, arg);
+	int ret;
+	
+	pm_runtime_forbid(dev->dev);
+	ret = drm_ioctl(filp, cmd, arg);
+	pm_runtime_allow(dev->dev);
+	return ret;
 	/* FIXME: do we need to wrap the other side of this */
 }
 
@@ -1141,8 +1133,12 @@ static void psb_remove(struct pci_dev *pdev)
 }
 
 static const struct dev_pm_ops psb_pm_ops = {
-	.resume = gma_power_resume,
 	.suspend = gma_power_suspend,
+	.resume = gma_power_resume,
+	.freeze = gma_power_suspend,
+	.thaw = gma_power_resume,
+	.poweroff = gma_power_suspend,
+	.restore = gma_power_resume,
 	.runtime_suspend = psb_runtime_suspend,
 	.runtime_resume = psb_runtime_resume,
 	.runtime_idle = psb_runtime_idle,
drivers/staging/gma500/psb_intel_display.c +0 -4
index 4afa671..caa9d86 100644
@@ -1102,10 +1102,6 @@ static int psb_crtc_set_config(struct drm_mode_set *set)
 {
 	int ret;
 	struct drm_device *dev = set->crtc->dev;
-	struct drm_psb_private *dev_priv = dev->dev_private;
-
-	if (!dev_priv->rpm_enabled)
-		return drm_crtc_helper_set_config(set);
 
 	pm_runtime_forbid(&dev->pdev->dev);
 	ret = drm_crtc_helper_set_config(set);