Supporting fused display configurations v5

Supporting fused display configurations v5

[Damien Lespiau]

Follow up of v4:
  http://lists.freedesktop.org/archives/intel-gfx/2014-January/037913.html

The major changes are:
  - we try to be cunning on CPT/PPT and look at the fuses lock bit of
    SFUSE_STRAP to detect if PCH display reads are being dropped
  - we provide a module parameter to forcefully disable display. This can help
    if we don't manage to detect the display is fused off.
  - INTEL_INFO() is still in use as it'd make compiling parts of the driver out
    easier (as Daniel showed)

-- 
Damien

[PATCH 1/8] drm/i915: Always use INTEL_INFO() to access the device_info structure

[Damien Lespiau]

If we make sure that all the dev_priv->info usages are wrapped by
INTEL_INFO(), we can easily modify the ->info field to be structure and
not a pointer while keeping the const protection in the INTEL_INFO()
macro.

v2: Rebased onto latest drm-nightly

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c      |  5 +++--
 drivers/gpu/drm/i915/i915_irq.c      |  4 ++--
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++------------
 drivers/gpu/drm/i915/intel_pm.c      | 13 +++++++++----
 4 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a8a069f..9b77be0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1014,7 +1014,8 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
 			struct timespec *timeout,
 			struct drm_i915_file_private *file_priv)
 {
-	drm_i915_private_t *dev_priv = ring->dev->dev_private;
+	struct drm_device *dev = ring->dev;
+	drm_i915_private_t *dev_priv = dev->dev_private;
 	const bool irq_test_in_progress =
 		ACCESS_ONCE(dev_priv->gpu_error.test_irq_rings) & intel_ring_flag(ring);
 	struct timespec before, now;
@@ -1029,7 +1030,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
 
 	timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
 
-	if (dev_priv->info->gen >= 6 && can_wait_boost(file_priv)) {
+	if (INTEL_INFO(dev)->gen >= 6 && can_wait_boost(file_priv)) {
 		gen6_rps_boost(dev_priv);
 		if (file_priv)
 			mod_delayed_work(dev_priv->wq,
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8f579bc..d4defd8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2276,7 +2276,7 @@ static int i915_enable_vblank(struct drm_device *dev, int pipe)
 				     PIPE_VBLANK_INTERRUPT_ENABLE);
 
 	/* maintain vblank delivery even in deep C-states */
-	if (dev_priv->info->gen == 3)
+	if (INTEL_INFO(dev)->gen == 3)
 		I915_WRITE(INSTPM, _MASKED_BIT_DISABLE(INSTPM_AGPBUSY_DIS));
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
@@ -2341,7 +2341,7 @@ static void i915_disable_vblank(struct drm_device *dev, int pipe)
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	if (dev_priv->info->gen == 3)
+	if (INTEL_INFO(dev)->gen == 3)
 		I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_AGPBUSY_DIS));
 
 	i915_disable_pipestat(dev_priv, pipe,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..1ad59d7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1030,7 +1030,7 @@ static void assert_fdi_tx_pll_enabled(struct drm_i915_private *dev_priv,
 	u32 val;
 
 	/* ILK FDI PLL is always enabled */
-	if (dev_priv->info->gen == 5)
+	if (INTEL_INFO(dev_priv->dev)->gen == 5)
 		return;
 
 	/* On Haswell, DDI ports are responsible for the FDI PLL setup */
@@ -1443,7 +1443,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
 	assert_pipe_disabled(dev_priv, crtc->pipe);
 
 	/* No really, not for ILK+ */
-	BUG_ON(dev_priv->info->gen >= 5);
+	BUG_ON(INTEL_INFO(dev)->gen >= 5);
 
 	/* PLL is protected by panel, make sure we can write it */
 	if (IS_MOBILE(dev) && !IS_I830(dev))
@@ -1549,11 +1549,12 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
  */
 static void ironlake_enable_shared_dpll(struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
 
 	/* PCH PLLs only available on ILK, SNB and IVB */
-	BUG_ON(dev_priv->info->gen < 5);
+	BUG_ON(INTEL_INFO(dev)->gen < 5);
 	if (WARN_ON(pll == NULL))
 		return;
 
@@ -1578,11 +1579,12 @@ static void ironlake_enable_shared_dpll(struct intel_crtc *crtc)
 
 static void intel_disable_shared_dpll(struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
 
 	/* PCH only available on ILK+ */
-	BUG_ON(dev_priv->info->gen < 5);
+	BUG_ON(INTEL_INFO(dev)->gen < 5);
 	if (WARN_ON(pll == NULL))
 	       return;
 
@@ -1617,7 +1619,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 	uint32_t reg, val, pipeconf_val;
 
 	/* PCH only available on ILK+ */
-	BUG_ON(dev_priv->info->gen < 5);
+	BUG_ON(INTEL_INFO(dev)->gen < 5);
 
 	/* Make sure PCH DPLL is enabled */
 	assert_shared_dpll_enabled(dev_priv,
@@ -1670,7 +1672,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 	u32 val, pipeconf_val;
 
 	/* PCH only available on ILK+ */
-	BUG_ON(dev_priv->info->gen < 5);
+	BUG_ON(INTEL_INFO(dev_priv->dev)->gen < 5);
 
 	/* FDI must be feeding us bits for PCH ports */
 	assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder);
@@ -1851,7 +1853,8 @@ static void intel_disable_pipe(struct drm_i915_private *dev_priv,
 void intel_flush_primary_plane(struct drm_i915_private *dev_priv,
 			       enum plane plane)
 {
-	u32 reg = dev_priv->info->gen >= 4 ? DSPSURF(plane) : DSPADDR(plane);
+	struct drm_device *dev = dev_priv->dev;
+	u32 reg = INTEL_INFO(dev)->gen >= 4 ? DSPSURF(plane) : DSPADDR(plane);
 
 	I915_WRITE(reg, I915_READ(reg));
 	POSTING_READ(reg);
@@ -7577,7 +7580,7 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 
 	/* we only need to pin inside GTT if cursor is non-phy */
 	mutex_lock(&dev->struct_mutex);
-	if (!dev_priv->info->cursor_needs_physical) {
+	if (!INTEL_INFO(dev)->cursor_needs_physical) {
 		unsigned alignment;
 
 		if (obj->tiling_mode) {
@@ -7625,7 +7628,7 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 
  finish:
 	if (intel_crtc->cursor_bo) {
-		if (dev_priv->info->cursor_needs_physical) {
+		if (INTEL_INFO(dev)->cursor_needs_physical) {
 			if (intel_crtc->cursor_bo != obj)
 				i915_gem_detach_phys_object(dev, intel_crtc->cursor_bo);
 		} else
@@ -8220,7 +8223,7 @@ void intel_mark_idle(struct drm_device *dev)
 		intel_decrease_pllclock(crtc);
 	}
 
-	if (dev_priv->info->gen >= 6)
+	if (INTEL_INFO(dev)->gen >= 6)
 		gen6_rps_idle(dev->dev_private);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f74d7f5..d24f92a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3903,9 +3903,10 @@ static unsigned long __i915_chipset_val(struct drm_i915_private *dev_priv)
 
 unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
 {
+	struct drm_device *dev = dev_priv->dev;
 	unsigned long val;
 
-	if (dev_priv->info->gen != 5)
+	if (INTEL_INFO(dev)->gen != 5)
 		return 0;
 
 	spin_lock_irq(&mchdev_lock);
@@ -3934,6 +3935,7 @@ unsigned long i915_mch_val(struct drm_i915_private *dev_priv)
 
 static u16 pvid_to_extvid(struct drm_i915_private *dev_priv, u8 pxvid)
 {
+	struct drm_device *dev = dev_priv->dev;
 	static const struct v_table {
 		u16 vd; /* in .1 mil */
 		u16 vm; /* in .1 mil */
@@ -4067,7 +4069,7 @@ static u16 pvid_to_extvid(struct drm_i915_private *dev_priv, u8 pxvid)
 		{ 16000, 14875, },
 		{ 16125, 15000, },
 	};
-	if (dev_priv->info->is_mobile)
+	if (INTEL_INFO(dev)->is_mobile)
 		return v_table[pxvid].vm;
 	else
 		return v_table[pxvid].vd;
@@ -4110,7 +4112,9 @@ static void __i915_update_gfx_val(struct drm_i915_private *dev_priv)
 
 void i915_update_gfx_val(struct drm_i915_private *dev_priv)
 {
-	if (dev_priv->info->gen != 5)
+	struct drm_device *dev = dev_priv->dev;
+
+	if (INTEL_INFO(dev)->gen != 5)
 		return;
 
 	spin_lock_irq(&mchdev_lock);
@@ -4159,9 +4163,10 @@ static unsigned long __i915_gfx_val(struct drm_i915_private *dev_priv)
 
 unsigned long i915_gfx_val(struct drm_i915_private *dev_priv)
 {
+	struct drm_device *dev = dev_priv->dev;
 	unsigned long val;
 
-	if (dev_priv->info->gen != 5)
+	if (INTEL_INFO(dev)->gen != 5)
 		return 0;
 
 	spin_lock_irq(&mchdev_lock);
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/8] drm/i915: Make the intel_device_info structure kept in dev_priv writable

[Damien Lespiau]

Turns out it'd be nice to change some device information at run-time or simply
have some code to fill in the info struct instead of having to declare the
values in 30+ structures.

What prompted this change is handling fused out display/pipe and tweaking
num_pipes at run-time, but I'm quite sure we'll find other flags/limits to
stick into dev_priv->info.

Most of the changes were done with a sed:
sed -i -e 's/dev_priv->info->/dev_priv->info./g' drivers/gpu/drm/i915/*[ch]

with a few tweaks to make it all work:
- Change the field definition in struct drm_i915_private
- adjust i915_dump_device_info()
- adjust i915_driver_load()
- adjust the INTEL_INFO() macro

v2: cast the info pointer returned by INTEL_INFO() to be const to catch
    uses that would modify the structure post-initialization.
    (Ville Syrjälä)

v3: Redo the patch onto latest drm-nightly,
    Keep the info field const to catch post initialization writes
    instead of the v2 solution,
    Use a direct structure copy for the initial info initialization to
    use the compiler type safety (Ville Syrjälä)

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v2)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v2)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |   9 ++-
 drivers/gpu/drm/i915/i915_drv.h |   4 +-
 drivers/gpu/drm/i915/i915_reg.h | 148 ++++++++++++++++++++--------------------
 3 files changed, 82 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 258b1be..0a9d0ad 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1442,7 +1442,7 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
 
 static void i915_dump_device_info(struct drm_i915_private *dev_priv)
 {
-	const struct intel_device_info *info = dev_priv->info;
+	const struct intel_device_info *info = &dev_priv->info;
 
 #define PRINT_S(name) "%s"
 #define SEP_EMPTY
@@ -1473,7 +1473,7 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
 int i915_driver_load(struct drm_device *dev, unsigned long flags)
 {
 	struct drm_i915_private *dev_priv;
-	struct intel_device_info *info;
+	struct intel_device_info *info, *device_info;
 	int ret = 0, mmio_bar, mmio_size;
 	uint32_t aperture_size;
 
@@ -1496,7 +1496,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	dev->dev_private = (void *)dev_priv;
 	dev_priv->dev = dev;
-	dev_priv->info = info;
+
+	/* copy initial configuration to dev_priv->info */
+	device_info = (struct intel_device_info *)&dev_priv->info;
+	*device_info = *info;
 
 	spin_lock_init(&dev_priv->irq_lock);
 	spin_lock_init(&dev_priv->gpu_error.lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..f66699f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1390,7 +1390,7 @@ typedef struct drm_i915_private {
 	struct drm_device *dev;
 	struct kmem_cache *slab;
 
-	const struct intel_device_info *info;
+	const struct intel_device_info info;
 
 	int relative_constants_mode;
 
@@ -1799,7 +1799,7 @@ struct drm_i915_file_private {
 	atomic_t rps_wait_boost;
 };
 
-#define INTEL_INFO(dev)	(to_i915(dev)->info)
+#define INTEL_INFO(dev)	(&to_i915(dev)->info)
 
 #define IS_I830(dev)		((dev)->pdev->device == 0x3577)
 #define IS_845G(dev)		((dev)->pdev->device == 0x2562)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cc3ea04..29b8819 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1204,8 +1204,8 @@
  */
 #define DPLL_A_OFFSET 0x6014
 #define DPLL_B_OFFSET 0x6018
-#define DPLL(pipe) (dev_priv->info->dpll_offsets[pipe] + \
-		    dev_priv->info->display_mmio_offset)
+#define DPLL(pipe) (dev_priv->info.dpll_offsets[pipe] + \
+		    dev_priv->info.display_mmio_offset)
 
 #define VGA0	0x6000
 #define VGA1	0x6004
@@ -1282,8 +1282,8 @@
 
 #define DPLL_A_MD_OFFSET 0x601c /* 965+ only */
 #define DPLL_B_MD_OFFSET 0x6020 /* 965+ only */
-#define DPLL_MD(pipe) (dev_priv->info->dpll_md_offsets[pipe] + \
-		       dev_priv->info->display_mmio_offset)
+#define DPLL_MD(pipe) (dev_priv->info.dpll_md_offsets[pipe] + \
+		       dev_priv->info.display_mmio_offset)
 
 /*
  * UDI pixel divider, controlling how many pixels are stuffed into a packet.
@@ -1352,7 +1352,7 @@
 #define  DSTATE_PLL_D3_OFF			(1<<3)
 #define  DSTATE_GFX_CLOCK_GATING		(1<<1)
 #define  DSTATE_DOT_CLOCK_GATING		(1<<0)
-#define DSPCLK_GATE_D	(dev_priv->info->display_mmio_offset + 0x6200)
+#define DSPCLK_GATE_D	(dev_priv->info.display_mmio_offset + 0x6200)
 # define DPUNIT_B_CLOCK_GATE_DISABLE		(1 << 30) /* 965 */
 # define VSUNIT_CLOCK_GATE_DISABLE		(1 << 29) /* 965 */
 # define VRHUNIT_CLOCK_GATE_DISABLE		(1 << 28) /* 965 */
@@ -1478,8 +1478,8 @@
  */
 #define PALETTE_A_OFFSET 0xa000
 #define PALETTE_B_OFFSET 0xa800
-#define PALETTE(pipe) (dev_priv->info->palette_offsets[pipe] + \
-		       dev_priv->info->display_mmio_offset)
+#define PALETTE(pipe) (dev_priv->info.palette_offsets[pipe] + \
+		       dev_priv->info.display_mmio_offset)
 
 /* MCH MMIO space */
 
@@ -1969,9 +1969,9 @@
 #define TRANSCODER_C_OFFSET 0x62000
 #define TRANSCODER_EDP_OFFSET 0x6f000
 
-#define _TRANSCODER2(pipe, reg) (dev_priv->info->trans_offsets[(pipe)] - \
-	dev_priv->info->trans_offsets[TRANSCODER_A] + (reg) + \
-	dev_priv->info->display_mmio_offset)
+#define _TRANSCODER2(pipe, reg) (dev_priv->info.trans_offsets[(pipe)] - \
+	dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \
+	dev_priv->info.display_mmio_offset)
 
 #define HTOTAL(trans) _TRANSCODER2(trans, _HTOTAL_A)
 #define HBLANK(trans) _TRANSCODER2(trans, _HBLANK_A)
@@ -2098,7 +2098,7 @@
 
 
 /* Hotplug control (945+ only) */
-#define PORT_HOTPLUG_EN		(dev_priv->info->display_mmio_offset + 0x61110)
+#define PORT_HOTPLUG_EN		(dev_priv->info.display_mmio_offset + 0x61110)
 #define   PORTB_HOTPLUG_INT_EN			(1 << 29)
 #define   PORTC_HOTPLUG_INT_EN			(1 << 28)
 #define   PORTD_HOTPLUG_INT_EN			(1 << 27)
@@ -2128,7 +2128,7 @@
 #define CRT_HOTPLUG_DETECT_VOLTAGE_325MV	(0 << 2)
 #define CRT_HOTPLUG_DETECT_VOLTAGE_475MV	(1 << 2)
 
-#define PORT_HOTPLUG_STAT	(dev_priv->info->display_mmio_offset + 0x61114)
+#define PORT_HOTPLUG_STAT	(dev_priv->info.display_mmio_offset + 0x61114)
 /*
  * HDMI/DP bits are gen4+
  *
@@ -2405,7 +2405,7 @@
 #define PP_DIVISOR	0x61210
 
 /* Panel fitting */
-#define PFIT_CONTROL	(dev_priv->info->display_mmio_offset + 0x61230)
+#define PFIT_CONTROL	(dev_priv->info.display_mmio_offset + 0x61230)
 #define   PFIT_ENABLE		(1 << 31)
 #define   PFIT_PIPE_MASK	(3 << 29)
 #define   PFIT_PIPE_SHIFT	29
@@ -2423,7 +2423,7 @@
 #define   PFIT_SCALING_PROGRAMMED (1 << 26)
 #define   PFIT_SCALING_PILLAR	(2 << 26)
 #define   PFIT_SCALING_LETTER	(3 << 26)
-#define PFIT_PGM_RATIOS	(dev_priv->info->display_mmio_offset + 0x61234)
+#define PFIT_PGM_RATIOS	(dev_priv->info.display_mmio_offset + 0x61234)
 /* Pre-965 */
 #define		PFIT_VERT_SCALE_SHIFT		20
 #define		PFIT_VERT_SCALE_MASK		0xfff00000
@@ -2435,25 +2435,25 @@
 #define		PFIT_HORIZ_SCALE_SHIFT_965	0
 #define		PFIT_HORIZ_SCALE_MASK_965	0x00001fff
 
-#define PFIT_AUTO_RATIOS (dev_priv->info->display_mmio_offset + 0x61238)
+#define PFIT_AUTO_RATIOS (dev_priv->info.display_mmio_offset + 0x61238)
 
-#define _VLV_BLC_PWM_CTL2_A (dev_priv->info->display_mmio_offset + 0x61250)
-#define _VLV_BLC_PWM_CTL2_B (dev_priv->info->display_mmio_offset + 0x61350)
+#define _VLV_BLC_PWM_CTL2_A (dev_priv->info.display_mmio_offset + 0x61250)
+#define _VLV_BLC_PWM_CTL2_B (dev_priv->info.display_mmio_offset + 0x61350)
 #define VLV_BLC_PWM_CTL2(pipe) _PIPE(pipe, _VLV_BLC_PWM_CTL2_A, \
 				     _VLV_BLC_PWM_CTL2_B)
 
-#define _VLV_BLC_PWM_CTL_A (dev_priv->info->display_mmio_offset + 0x61254)
-#define _VLV_BLC_PWM_CTL_B (dev_priv->info->display_mmio_offset + 0x61354)
+#define _VLV_BLC_PWM_CTL_A (dev_priv->info.display_mmio_offset + 0x61254)
+#define _VLV_BLC_PWM_CTL_B (dev_priv->info.display_mmio_offset + 0x61354)
 #define VLV_BLC_PWM_CTL(pipe) _PIPE(pipe, _VLV_BLC_PWM_CTL_A, \
 				    _VLV_BLC_PWM_CTL_B)
 
-#define _VLV_BLC_HIST_CTL_A (dev_priv->info->display_mmio_offset + 0x61260)
-#define _VLV_BLC_HIST_CTL_B (dev_priv->info->display_mmio_offset + 0x61360)
+#define _VLV_BLC_HIST_CTL_A (dev_priv->info.display_mmio_offset + 0x61260)
+#define _VLV_BLC_HIST_CTL_B (dev_priv->info.display_mmio_offset + 0x61360)
 #define VLV_BLC_HIST_CTL(pipe) _PIPE(pipe, _VLV_BLC_HIST_CTL_A, \
 				     _VLV_BLC_HIST_CTL_B)
 
 /* Backlight control */
-#define BLC_PWM_CTL2	(dev_priv->info->display_mmio_offset + 0x61250) /* 965+ only */
+#define BLC_PWM_CTL2	(dev_priv->info.display_mmio_offset + 0x61250) /* 965+ only */
 #define   BLM_PWM_ENABLE		(1 << 31)
 #define   BLM_COMBINATION_MODE		(1 << 30) /* gen4 only */
 #define   BLM_PIPE_SELECT		(1 << 29)
@@ -2476,7 +2476,7 @@
 #define   BLM_PHASE_IN_COUNT_MASK	(0xff << 8)
 #define   BLM_PHASE_IN_INCR_SHIFT	(0)
 #define   BLM_PHASE_IN_INCR_MASK	(0xff << 0)
-#define BLC_PWM_CTL	(dev_priv->info->display_mmio_offset + 0x61254)
+#define BLC_PWM_CTL	(dev_priv->info.display_mmio_offset + 0x61254)
 /*
  * This is the most significant 15 bits of the number of backlight cycles in a
  * complete cycle of the modulated backlight control.
@@ -2498,7 +2498,7 @@
 #define   BACKLIGHT_DUTY_CYCLE_MASK_PNV		(0xfffe)
 #define   BLM_POLARITY_PNV			(1 << 0) /* pnv only */
 
-#define BLC_HIST_CTL	(dev_priv->info->display_mmio_offset + 0x61260)
+#define BLC_HIST_CTL	(dev_priv->info.display_mmio_offset + 0x61260)
 
 /* New registers for PCH-split platforms. Safe where new bits show up, the
  * register layout machtes with gen4 BLC_PWM_CTL[12]. */
@@ -3287,9 +3287,9 @@
  */
 #define PIPE_EDP_OFFSET	0x7f000
 
-#define _PIPE2(pipe, reg) (dev_priv->info->pipe_offsets[pipe] - \
-	dev_priv->info->pipe_offsets[PIPE_A] + (reg) + \
-	dev_priv->info->display_mmio_offset)
+#define _PIPE2(pipe, reg) (dev_priv->info.pipe_offsets[pipe] - \
+	dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \
+	dev_priv->info.display_mmio_offset)
 
 #define PIPECONF(pipe) _PIPE2(pipe, _PIPEACONF)
 #define PIPEDSL(pipe)  _PIPE2(pipe, _PIPEADSL)
@@ -3351,7 +3351,7 @@
 #define   DSPARB_BEND_SHIFT	9 /* on 855 */
 #define   DSPARB_AEND_SHIFT	0
 
-#define DSPFW1			(dev_priv->info->display_mmio_offset + 0x70034)
+#define DSPFW1			(dev_priv->info.display_mmio_offset + 0x70034)
 #define   DSPFW_SR_SHIFT	23
 #define   DSPFW_SR_MASK		(0x1ff<<23)
 #define   DSPFW_CURSORB_SHIFT	16
@@ -3359,11 +3359,11 @@
 #define   DSPFW_PLANEB_SHIFT	8
 #define   DSPFW_PLANEB_MASK	(0x7f<<8)
 #define   DSPFW_PLANEA_MASK	(0x7f)
-#define DSPFW2			(dev_priv->info->display_mmio_offset + 0x70038)
+#define DSPFW2			(dev_priv->info.display_mmio_offset + 0x70038)
 #define   DSPFW_CURSORA_MASK	0x00003f00
 #define   DSPFW_CURSORA_SHIFT	8
 #define   DSPFW_PLANEC_MASK	(0x7f)
-#define DSPFW3			(dev_priv->info->display_mmio_offset + 0x7003c)
+#define DSPFW3			(dev_priv->info.display_mmio_offset + 0x7003c)
 #define   DSPFW_HPLL_SR_EN	(1<<31)
 #define   DSPFW_CURSOR_SR_SHIFT	24
 #define   PINEVIEW_SELF_REFRESH_EN	(1<<30)
@@ -3371,8 +3371,8 @@
 #define   DSPFW_HPLL_CURSOR_SHIFT	16
 #define   DSPFW_HPLL_CURSOR_MASK	(0x3f<<16)
 #define   DSPFW_HPLL_SR_MASK		(0x1ff)
-#define DSPFW4			(dev_priv->info->display_mmio_offset + 0x70070)
-#define DSPFW7			(dev_priv->info->display_mmio_offset + 0x7007c)
+#define DSPFW4			(dev_priv->info.display_mmio_offset + 0x70070)
+#define DSPFW7			(dev_priv->info.display_mmio_offset + 0x7007c)
 
 /* drain latency register values*/
 #define DRAIN_LATENCY_PRECISION_32	32
@@ -3496,12 +3496,12 @@
 #define   PIPE_PIXEL_MASK         0x00ffffff
 #define   PIPE_PIXEL_SHIFT        0
 /* GM45+ just has to be different */
-#define _PIPEA_FRMCOUNT_GM45	(dev_priv->info->display_mmio_offset + 0x70040)
-#define _PIPEA_FLIPCOUNT_GM45	(dev_priv->info->display_mmio_offset + 0x70044)
+#define _PIPEA_FRMCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x70040)
+#define _PIPEA_FLIPCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x70044)
 #define PIPE_FRMCOUNT_GM45(pipe) _PIPE(pipe, _PIPEA_FRMCOUNT_GM45, _PIPEB_FRMCOUNT_GM45)
 
 /* Cursor A & B regs */
-#define _CURACNTR		(dev_priv->info->display_mmio_offset + 0x70080)
+#define _CURACNTR		(dev_priv->info.display_mmio_offset + 0x70080)
 /* Old style CUR*CNTR flags (desktop 8xx) */
 #define   CURSOR_ENABLE		0x80000000
 #define   CURSOR_GAMMA_ENABLE	0x40000000
@@ -3524,16 +3524,16 @@
 #define   MCURSOR_PIPE_B	(1 << 28)
 #define   MCURSOR_GAMMA_ENABLE  (1 << 26)
 #define   CURSOR_TRICKLE_FEED_DISABLE	(1 << 14)
-#define _CURABASE		(dev_priv->info->display_mmio_offset + 0x70084)
-#define _CURAPOS		(dev_priv->info->display_mmio_offset + 0x70088)
+#define _CURABASE		(dev_priv->info.display_mmio_offset + 0x70084)
+#define _CURAPOS		(dev_priv->info.display_mmio_offset + 0x70088)
 #define   CURSOR_POS_MASK       0x007FF
 #define   CURSOR_POS_SIGN       0x8000
 #define   CURSOR_X_SHIFT        0
 #define   CURSOR_Y_SHIFT        16
 #define CURSIZE			0x700a0
-#define _CURBCNTR		(dev_priv->info->display_mmio_offset + 0x700c0)
-#define _CURBBASE		(dev_priv->info->display_mmio_offset + 0x700c4)
-#define _CURBPOS		(dev_priv->info->display_mmio_offset + 0x700c8)
+#define _CURBCNTR		(dev_priv->info.display_mmio_offset + 0x700c0)
+#define _CURBBASE		(dev_priv->info.display_mmio_offset + 0x700c4)
+#define _CURBPOS		(dev_priv->info.display_mmio_offset + 0x700c8)
 
 #define _CURBCNTR_IVB		0x71080
 #define _CURBBASE_IVB		0x71084
@@ -3608,44 +3608,44 @@
 #define I915_HI_DISPBASE(val)	(val & DISP_BASEADDR_MASK)
 
 /* VBIOS flags */
-#define SWF00			(dev_priv->info->display_mmio_offset + 0x71410)
-#define SWF01			(dev_priv->info->display_mmio_offset + 0x71414)
-#define SWF02			(dev_priv->info->display_mmio_offset + 0x71418)
-#define SWF03			(dev_priv->info->display_mmio_offset + 0x7141c)
-#define SWF04			(dev_priv->info->display_mmio_offset + 0x71420)
-#define SWF05			(dev_priv->info->display_mmio_offset + 0x71424)
-#define SWF06			(dev_priv->info->display_mmio_offset + 0x71428)
-#define SWF10			(dev_priv->info->display_mmio_offset + 0x70410)
-#define SWF11			(dev_priv->info->display_mmio_offset + 0x70414)
-#define SWF14			(dev_priv->info->display_mmio_offset + 0x71420)
-#define SWF30			(dev_priv->info->display_mmio_offset + 0x72414)
-#define SWF31			(dev_priv->info->display_mmio_offset + 0x72418)
-#define SWF32			(dev_priv->info->display_mmio_offset + 0x7241c)
+#define SWF00			(dev_priv->info.display_mmio_offset + 0x71410)
+#define SWF01			(dev_priv->info.display_mmio_offset + 0x71414)
+#define SWF02			(dev_priv->info.display_mmio_offset + 0x71418)
+#define SWF03			(dev_priv->info.display_mmio_offset + 0x7141c)
+#define SWF04			(dev_priv->info.display_mmio_offset + 0x71420)
+#define SWF05			(dev_priv->info.display_mmio_offset + 0x71424)
+#define SWF06			(dev_priv->info.display_mmio_offset + 0x71428)
+#define SWF10			(dev_priv->info.display_mmio_offset + 0x70410)
+#define SWF11			(dev_priv->info.display_mmio_offset + 0x70414)
+#define SWF14			(dev_priv->info.display_mmio_offset + 0x71420)
+#define SWF30			(dev_priv->info.display_mmio_offset + 0x72414)
+#define SWF31			(dev_priv->info.display_mmio_offset + 0x72418)
+#define SWF32			(dev_priv->info.display_mmio_offset + 0x7241c)
 
 /* Pipe B */
-#define _PIPEBDSL		(dev_priv->info->display_mmio_offset + 0x71000)
-#define _PIPEBCONF		(dev_priv->info->display_mmio_offset + 0x71008)
-#define _PIPEBSTAT		(dev_priv->info->display_mmio_offset + 0x71024)
+#define _PIPEBDSL		(dev_priv->info.display_mmio_offset + 0x71000)
+#define _PIPEBCONF		(dev_priv->info.display_mmio_offset + 0x71008)
+#define _PIPEBSTAT		(dev_priv->info.display_mmio_offset + 0x71024)
 #define _PIPEBFRAMEHIGH		0x71040
 #define _PIPEBFRAMEPIXEL	0x71044
-#define _PIPEB_FRMCOUNT_GM45	(dev_priv->info->display_mmio_offset + 0x71040)
-#define _PIPEB_FLIPCOUNT_GM45	(dev_priv->info->display_mmio_offset + 0x71044)
+#define _PIPEB_FRMCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x71040)
+#define _PIPEB_FLIPCOUNT_GM45	(dev_priv->info.display_mmio_offset + 0x71044)
 
 
 /* Display B control */
-#define _DSPBCNTR		(dev_priv->info->display_mmio_offset + 0x71180)
+#define _DSPBCNTR		(dev_priv->info.display_mmio_offset + 0x71180)
 #define   DISPPLANE_ALPHA_TRANS_ENABLE		(1<<15)
 #define   DISPPLANE_ALPHA_TRANS_DISABLE		0
 #define   DISPPLANE_SPRITE_ABOVE_DISPLAY	0
 #define   DISPPLANE_SPRITE_ABOVE_OVERLAY	(1)
-#define _DSPBADDR		(dev_priv->info->display_mmio_offset + 0x71184)
-#define _DSPBSTRIDE		(dev_priv->info->display_mmio_offset + 0x71188)
-#define _DSPBPOS		(dev_priv->info->display_mmio_offset + 0x7118C)
-#define _DSPBSIZE		(dev_priv->info->display_mmio_offset + 0x71190)
-#define _DSPBSURF		(dev_priv->info->display_mmio_offset + 0x7119C)
-#define _DSPBTILEOFF		(dev_priv->info->display_mmio_offset + 0x711A4)
-#define _DSPBOFFSET		(dev_priv->info->display_mmio_offset + 0x711A4)
-#define _DSPBSURFLIVE		(dev_priv->info->display_mmio_offset + 0x711AC)
+#define _DSPBADDR		(dev_priv->info.display_mmio_offset + 0x71184)
+#define _DSPBSTRIDE		(dev_priv->info.display_mmio_offset + 0x71188)
+#define _DSPBPOS		(dev_priv->info.display_mmio_offset + 0x7118C)
+#define _DSPBSIZE		(dev_priv->info.display_mmio_offset + 0x71190)
+#define _DSPBSURF		(dev_priv->info.display_mmio_offset + 0x7119C)
+#define _DSPBTILEOFF		(dev_priv->info.display_mmio_offset + 0x711A4)
+#define _DSPBOFFSET		(dev_priv->info.display_mmio_offset + 0x711A4)
+#define _DSPBSURFLIVE		(dev_priv->info.display_mmio_offset + 0x711AC)
 
 /* Sprite A control */
 #define _DVSACNTR		0x72180
@@ -5051,7 +5051,7 @@
 #define   GEN8_CENTROID_PIXEL_OPT_DIS	(1<<8)
 #define   GEN8_SAMPLER_POWER_BYPASS_DIS	(1<<1)
 
-#define G4X_AUD_VID_DID			(dev_priv->info->display_mmio_offset + 0x62020)
+#define G4X_AUD_VID_DID			(dev_priv->info.display_mmio_offset + 0x62020)
 #define INTEL_AUDIO_DEVCL		0x808629FB
 #define INTEL_AUDIO_DEVBLC		0x80862801
 #define INTEL_AUDIO_DEVCTG		0x80862802
@@ -5904,11 +5904,11 @@
 #define  READ_DATA_VALID(n)				(1 << (n))
 
 /* For UMS only (deprecated): */
-#define _PALETTE_A (dev_priv->info->display_mmio_offset + 0xa000)
-#define _PALETTE_B (dev_priv->info->display_mmio_offset + 0xa800)
-#define _DPLL_A (dev_priv->info->display_mmio_offset + 0x6014)
-#define _DPLL_B (dev_priv->info->display_mmio_offset + 0x6018)
-#define _DPLL_A_MD (dev_priv->info->display_mmio_offset + 0x601c)
-#define _DPLL_B_MD (dev_priv->info->display_mmio_offset + 0x6020)
+#define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
+#define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
+#define _DPLL_A (dev_priv->info.display_mmio_offset + 0x6014)
+#define _DPLL_B (dev_priv->info.display_mmio_offset + 0x6018)
+#define _DPLL_A_MD (dev_priv->info.display_mmio_offset + 0x601c)
+#define _DPLL_B_MD (dev_priv->info.display_mmio_offset + 0x6020)
 
 #endif /* _I915_REG_H_ */
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/8] drm/i915: Make the intel_device_info structure kept in dev_priv writable

[Daniel Vetter]

On Fri, Feb 07, 2014 at 07:12:48PM +0000, Damien Lespiau wrote:
> Turns out it'd be nice to change some device information at run-time or simply
> have some code to fill in the info struct instead of having to declare the
> values in 30+ structures.
> 
> What prompted this change is handling fused out display/pipe and tweaking
> num_pipes at run-time, but I'm quite sure we'll find other flags/limits to
> stick into dev_priv->info.
> 
> Most of the changes were done with a sed:
> sed -i -e 's/dev_priv->info->/dev_priv->info./g' drivers/gpu/drm/i915/*[ch]
> 
> with a few tweaks to make it all work:
> - Change the field definition in struct drm_i915_private
> - adjust i915_dump_device_info()
> - adjust i915_driver_load()
> - adjust the INTEL_INFO() macro
> 
> v2: cast the info pointer returned by INTEL_INFO() to be const to catch
>     uses that would modify the structure post-initialization.
>     (Ville Syrjälä)
> 
> v3: Redo the patch onto latest drm-nightly,
>     Keep the info field const to catch post initialization writes
>     instead of the v2 solution,
>     Use a direct structure copy for the initial info initialization to
>     use the compiler type safety (Ville Syrjälä)
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v2)
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v2)
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

[snip]

> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 728b9c3..f66699f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1390,7 +1390,7 @@ typedef struct drm_i915_private {
>  	struct drm_device *dev;
>  	struct kmem_cache *slab;
>  
> -	const struct intel_device_info *info;
> +	const struct intel_device_info info;

Since every access should now go through the macro I think it'd be good to
give this a __ prefix to make it clear that users better think twice
before using it. Maybe as a patch on top of all this?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/8] drm/i915: Make the intel_device_info structure kept in dev_priv writable

[Jani Nikula]

On Mon, 10 Feb 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Feb 07, 2014 at 07:12:48PM +0000, Damien Lespiau wrote:
>> Turns out it'd be nice to change some device information at run-time or simply
>> have some code to fill in the info struct instead of having to declare the
>> values in 30+ structures.
>> 
>> What prompted this change is handling fused out display/pipe and tweaking
>> num_pipes at run-time, but I'm quite sure we'll find other flags/limits to
>> stick into dev_priv->info.
>> 
>> Most of the changes were done with a sed:
>> sed -i -e 's/dev_priv->info->/dev_priv->info./g' drivers/gpu/drm/i915/*[ch]
>> 
>> with a few tweaks to make it all work:
>> - Change the field definition in struct drm_i915_private
>> - adjust i915_dump_device_info()
>> - adjust i915_driver_load()
>> - adjust the INTEL_INFO() macro
>> 
>> v2: cast the info pointer returned by INTEL_INFO() to be const to catch
>>     uses that would modify the structure post-initialization.
>>     (Ville Syrjälä)
>> 
>> v3: Redo the patch onto latest drm-nightly,
>>     Keep the info field const to catch post initialization writes
>>     instead of the v2 solution,
>>     Use a direct structure copy for the initial info initialization to
>>     use the compiler type safety (Ville Syrjälä)
>> 
>> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v2)
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v2)
>> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
>
> [snip]
>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 728b9c3..f66699f 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1390,7 +1390,7 @@ typedef struct drm_i915_private {
>>  	struct drm_device *dev;
>>  	struct kmem_cache *slab;
>>  
>> -	const struct intel_device_info *info;
>> +	const struct intel_device_info info;
>
> Since every access should now go through the macro I think it'd be good to
> give this a __ prefix to make it clear that users better think twice
> before using it. Maybe as a patch on top of all this?

Begs the question whether every access also in i915_reg.h should be
through the macro. If that was changed before this patch, this one
wouldn't have to touch i915_reg.h at all AFAICT.

BR,
Jani.


> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/8] drm/i915: Make the intel_device_info structure kept in dev_priv writable

[Ville Syrjälä]

On Mon, Feb 10, 2014 at 10:09:11AM +0100, Daniel Vetter wrote:
> On Fri, Feb 07, 2014 at 07:12:48PM +0000, Damien Lespiau wrote:
> > Turns out it'd be nice to change some device information at run-time or simply
> > have some code to fill in the info struct instead of having to declare the
> > values in 30+ structures.
> > 
> > What prompted this change is handling fused out display/pipe and tweaking
> > num_pipes at run-time, but I'm quite sure we'll find other flags/limits to
> > stick into dev_priv->info.
> > 
> > Most of the changes were done with a sed:
> > sed -i -e 's/dev_priv->info->/dev_priv->info./g' drivers/gpu/drm/i915/*[ch]
> > 
> > with a few tweaks to make it all work:
> > - Change the field definition in struct drm_i915_private
> > - adjust i915_dump_device_info()
> > - adjust i915_driver_load()
> > - adjust the INTEL_INFO() macro
> > 
> > v2: cast the info pointer returned by INTEL_INFO() to be const to catch
> >     uses that would modify the structure post-initialization.
> >     (Ville Syrjälä)
> > 
> > v3: Redo the patch onto latest drm-nightly,
> >     Keep the info field const to catch post initialization writes
> >     instead of the v2 solution,
> >     Use a direct structure copy for the initial info initialization to
> >     use the compiler type safety (Ville Syrjälä)
> > 
> > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v2)
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v2)
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> [snip]
> 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 728b9c3..f66699f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1390,7 +1390,7 @@ typedef struct drm_i915_private {
> >  	struct drm_device *dev;
> >  	struct kmem_cache *slab;
> >  
> > -	const struct intel_device_info *info;
> > +	const struct intel_device_info info;
> 
> Since every access should now go through the macro I think it'd be good to
> give this a __ prefix to make it clear that users better think twice
> before using it. Maybe as a patch on top of all this?

No. Everyone having to use the macro was a requirement of the v2 patch.
With v3 that requirement was lifted since the const is right there on
the struct itself. I think that was the whole point of v3.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 2/8] drm/i915: Make the intel_device_info structure kept in dev_priv writable

[Damien Lespiau]

On Mon, Feb 10, 2014 at 02:53:29PM +0200, Ville Syrjälä wrote:
> > Since every access should now go through the macro I think it'd be good to
> > give this a __ prefix to make it clear that users better think twice
> > before using it. Maybe as a patch on top of all this?
> 
> No. Everyone having to use the macro was a requirement of the v2 patch.
> With v3 that requirement was lifted since the const is right there on
> the struct itself. I think that was the whole point of v3.

The "everyone should now go through the macro" is not for the const.
There's a new idea floating around to replace the macros by hardcoded
values to be able to compile the driver for a specific platform and use
the compiler dead code elimiation pass(es) to reduce the text size.

There may be more cunning ways to reduce the driver size, haven't
thought much about it.

-- 
Damien

Re: [PATCH 2/8] drm/i915: Make the intel_device_info structure kept in dev_priv writable

[Ville Syrjälä]

On Mon, Feb 10, 2014 at 02:06:16PM +0000, Damien Lespiau wrote:
> On Mon, Feb 10, 2014 at 02:53:29PM +0200, Ville Syrjälä wrote:
> > > Since every access should now go through the macro I think it'd be good to
> > > give this a __ prefix to make it clear that users better think twice
> > > before using it. Maybe as a patch on top of all this?
> > 
> > No. Everyone having to use the macro was a requirement of the v2 patch.
> > With v3 that requirement was lifted since the const is right there on
> > the struct itself. I think that was the whole point of v3.
> 
> The "everyone should now go through the macro" is not for the const.
> There's a new idea floating around to replace the macros by hardcoded
> values to be able to compile the driver for a specific platform and use
> the compiler dead code elimiation pass(es) to reduce the text size.
> 
> There may be more cunning ways to reduce the driver size, haven't
> thought much about it.

One easy thing to do would be just dropping the unused output types
from the driver when doing the special build. I'd be surprised if
the compiler is smart enough to figure that stuff out with the
function pointer indirections we do there. Not sure how much we'd
save though.

Or just make kernel memory swappable ;)

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 2/8] drm/i915: Make the intel_device_info structure kept in dev_priv writable

[Daniel Vetter]

On Mon, Feb 10, 2014 at 04:17:10PM +0200, Ville Syrjälä wrote:
> On Mon, Feb 10, 2014 at 02:06:16PM +0000, Damien Lespiau wrote:
> > On Mon, Feb 10, 2014 at 02:53:29PM +0200, Ville Syrjälä wrote:
> > > > Since every access should now go through the macro I think it'd be good to
> > > > give this a __ prefix to make it clear that users better think twice
> > > > before using it. Maybe as a patch on top of all this?
> > > 
> > > No. Everyone having to use the macro was a requirement of the v2 patch.
> > > With v3 that requirement was lifted since the const is right there on
> > > the struct itself. I think that was the whole point of v3.
> > 
> > The "everyone should now go through the macro" is not for the const.
> > There's a new idea floating around to replace the macros by hardcoded
> > values to be able to compile the driver for a specific platform and use
> > the compiler dead code elimiation pass(es) to reduce the text size.
> > 
> > There may be more cunning ways to reduce the driver size, haven't
> > thought much about it.
> 
> One easy thing to do would be just dropping the unused output types
> from the driver when doing the special build. I'd be surprised if
> the compiler is smart enough to figure that stuff out with the
> function pointer indirections we do there. Not sure how much we'd
> save though.

It is (mostly) smart enough to rip it all out. But for completely unclear
reasons (at least to me) it doesn't do the right thing with ripping out
dead code from intel_setup_outputs, so you have to help it out with the
occasionaly && 0. Even thought the INTEL_INFO check hardcodes to the
same ... But maybe I've missed something.

The big problem is the lack of lto - I've done these experiments by
including all our .c files into one big thing. Compile time went through
the roof a bit ;-)

> Or just make kernel memory swappable ;)

*shudder*

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/8] drm/i915: Make the intel_device_info structure kept in dev_priv writable

[Daniel Vetter]

On Mon, Feb 10, 2014 at 02:53:29PM +0200, Ville Syrjälä wrote:
> On Mon, Feb 10, 2014 at 10:09:11AM +0100, Daniel Vetter wrote:
> > On Fri, Feb 07, 2014 at 07:12:48PM +0000, Damien Lespiau wrote:
> > > Turns out it'd be nice to change some device information at run-time or simply
> > > have some code to fill in the info struct instead of having to declare the
> > > values in 30+ structures.
> > > 
> > > What prompted this change is handling fused out display/pipe and tweaking
> > > num_pipes at run-time, but I'm quite sure we'll find other flags/limits to
> > > stick into dev_priv->info.
> > > 
> > > Most of the changes were done with a sed:
> > > sed -i -e 's/dev_priv->info->/dev_priv->info./g' drivers/gpu/drm/i915/*[ch]
> > > 
> > > with a few tweaks to make it all work:
> > > - Change the field definition in struct drm_i915_private
> > > - adjust i915_dump_device_info()
> > > - adjust i915_driver_load()
> > > - adjust the INTEL_INFO() macro
> > > 
> > > v2: cast the info pointer returned by INTEL_INFO() to be const to catch
> > >     uses that would modify the structure post-initialization.
> > >     (Ville Syrjälä)
> > > 
> > > v3: Redo the patch onto latest drm-nightly,
> > >     Keep the info field const to catch post initialization writes
> > >     instead of the v2 solution,
> > >     Use a direct structure copy for the initial info initialization to
> > >     use the compiler type safety (Ville Syrjälä)
> > > 
> > > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v2)
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v2)
> > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > 
> > [snip]
> > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 728b9c3..f66699f 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1390,7 +1390,7 @@ typedef struct drm_i915_private {
> > >  	struct drm_device *dev;
> > >  	struct kmem_cache *slab;
> > >  
> > > -	const struct intel_device_info *info;
> > > +	const struct intel_device_info info;
> > 
> > Since every access should now go through the macro I think it'd be good to
> > give this a __ prefix to make it clear that users better think twice
> > before using it. Maybe as a patch on top of all this?
> 
> No. Everyone having to use the macro was a requirement of the v2 patch.
> With v3 that requirement was lifted since the const is right there on
> the struct itself. I think that was the whole point of v3.

Oops, I've missed that v3 has the const even on the dev_priv->info pointer
now. No need for any gratious rename fests then imo.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 3/8] drm/i915: Move num_plane to the intel_device_info structure

[Damien Lespiau]

And rename it to num_sprites as this value doesn't count the primary
plane.

This limit lives with num_pipes really, and now that dev_priv->info is
writable we can put it there instead.

While at it, introduce a intel_device_info_runtime_init() where we'll be
able to gather the device info fields at run-time.

v2: rename num_plane to num_sprites (Ville Syrjälä)
v3: rebase on top of latest drm-nightly

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v2)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v2)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      | 23 ++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_drv.h      |  5 ++---
 drivers/gpu/drm/i915/intel_display.c |  4 ++--
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0a9d0ad..e281298 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1459,6 +1459,25 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
 #undef SEP_COMMA
 }
 
+/*
+ * Determine various intel_device_info fields at runtime.
+ *
+ * Use it when either:
+ *   - it's judged too laborious to fill n static structures with the limit
+ *     when a simple if statement does the job,
+ *   - run-time checks (eg read fuse/strap registers) are needed.
+ */
+static void intel_device_info_runtime_init(struct drm_device *dev)
+{
+	struct intel_device_info *info;
+
+	info = (struct intel_device_info *)&to_i915(dev)->info;
+
+	info->num_sprites = 1;
+	if (IS_VALLEYVIEW(dev))
+		info->num_sprites = 2;
+}
+
 /**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
@@ -1638,9 +1657,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (!IS_I945G(dev) && !IS_I945GM(dev))
 		pci_enable_msi(dev->pdev);
 
-	dev_priv->num_plane = 1;
-	if (IS_VALLEYVIEW(dev))
-		dev_priv->num_plane = 2;
+	intel_device_info_runtime_init(dev);
 
 	if (INTEL_INFO(dev)->num_pipes) {
 		ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f66699f..36ea189 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -79,7 +79,7 @@ enum plane {
 };
 #define plane_name(p) ((p) + 'A')
 
-#define sprite_name(p, s) ((p) * dev_priv->num_plane + (s) + 'A')
+#define sprite_name(p, s) ((p) * INTEL_INFO(dev)->num_sprites + (s) + 'A')
 
 enum port {
 	PORT_A = 0,
@@ -530,6 +530,7 @@ struct intel_uncore {
 struct intel_device_info {
 	u32 display_mmio_offset;
 	u8 num_pipes:3;
+	u8 num_sprites:2;
 	u8 gen;
 	u8 ring_mask; /* Rings supported by the HW */
 	DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
@@ -1450,8 +1451,6 @@ typedef struct drm_i915_private {
 	u32 hpd_event_bits;
 	struct timer_list hotplug_reenable_timer;
 
-	int num_plane;
-
 	struct i915_fbc fbc;
 	struct intel_opregion opregion;
 	struct intel_vbt_data vbt;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1ad59d7..c2686d7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1189,7 +1189,7 @@ static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
 	u32 val;
 
 	if (IS_VALLEYVIEW(dev)) {
-		for (i = 0; i < dev_priv->num_plane; i++) {
+		for (i = 0; i < INTEL_INFO(dev)->num_sprites; i++) {
 			reg = SPCNTR(pipe, i);
 			val = I915_READ(reg);
 			WARN((val & SP_ENABLE),
@@ -11038,7 +11038,7 @@ void intel_modeset_init(struct drm_device *dev)
 
 	for_each_pipe(i) {
 		intel_crtc_init(dev, i);
-		for (j = 0; j < dev_priv->num_plane; j++) {
+		for (j = 0; j < INTEL_INFO(dev)->num_sprites; j++) {
 			ret = intel_plane_init(dev, i, j);
 			if (ret)
 				DRM_DEBUG_KMS("pipe %c sprite %c init failed: %d\n",
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/8] drm/i915: Consolidate FUSE_STRAP in one set of defines

[Damien Lespiau]

We had 2 set of defines for the same register, so make it one.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 18 ++++++++----------
 drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c |  3 +--
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 29b8819..50968ef 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4109,13 +4109,14 @@
 #define  ILK_ELPIN_409_SELECT	(1 << 25)
 #define  ILK_DPARB_GATE	(1<<22)
 #define  ILK_VSDPFD_FULL	(1<<21)
-#define ILK_DISPLAY_CHICKEN_FUSES	0x42014
-#define  ILK_INTERNAL_GRAPHICS_DISABLE	(1<<31)
-#define  ILK_INTERNAL_DISPLAY_DISABLE	(1<<30)
-#define  ILK_DISPLAY_DEBUG_DISABLE	(1<<29)
-#define  ILK_HDCP_DISABLE		(1<<25)
-#define  ILK_eDP_A_DISABLE		(1<<24)
-#define  ILK_DESKTOP			(1<<23)
+#define FUSE_STRAP			0x42014
+#define  ILK_INTERNAL_GRAPHICS_DISABLE	(1 << 31)
+#define  ILK_INTERNAL_DISPLAY_DISABLE	(1 << 30)
+#define  ILK_DISPLAY_DEBUG_DISABLE	(1 << 29)
+#define  ILK_HDCP_DISABLE		(1 << 25)
+#define  ILK_eDP_A_DISABLE		(1 << 24)
+#define  HSW_CDCLK_LIMIT		(1 << 24)
+#define  ILK_DESKTOP			(1 << 23)
 
 #define ILK_DSPCLK_GATE_D			0x42020
 #define   ILK_VRHUNIT_CLOCK_GATE_DISABLE	(1 << 28)
@@ -4178,9 +4179,6 @@
 #define HSW_SCRATCH1				0xb038
 #define  HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE	(1<<27)
 
-#define HSW_FUSE_STRAP		0x42014
-#define  HSW_CDCLK_LIMIT	(1 << 24)
-
 /* PCH */
 
 /* south display engine interrupt: IBX */
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cd65dd0..2643d3b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1415,7 +1415,7 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
 
 	if (lcpll & LCPLL_CD_SOURCE_FCLK) {
 		return 800000;
-	} else if (I915_READ(HSW_FUSE_STRAP) & HSW_CDCLK_LIMIT) {
+	} else if (I915_READ(FUSE_STRAP) & HSW_CDCLK_LIMIT) {
 		return 450000;
 	} else if (freq == LCPLL_CLK_FREQ_450) {
 		return 450000;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c2686d7..807e815 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10373,8 +10373,7 @@ static bool has_edp_a(struct drm_device *dev)
 	if ((I915_READ(DP_A) & DP_DETECTED) == 0)
 		return false;
 
-	if (IS_GEN5(dev) &&
-	    (I915_READ(ILK_DISPLAY_CHICKEN_FUSES) & ILK_eDP_A_DISABLE))
+	if (IS_GEN5(dev) && (I915_READ(FUSE_STRAP) & ILK_eDP_A_DISABLE))
 		return false;
 
 	return true;
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/8] drm/i915: Disable display when fused off

[Damien Lespiau]

FUSE_STRAP has a bit to inform us that the display has been fused off.
Use it to setup the definitive number of pipes at run-time.

v2: actually tweak num_pipes, not num_planes
v3: also tests SFUSE_STRAP bit 7
v4: rebase on top of drm-nightly
    use DRM_INFO() for the message telling display is fused off
    try to read the FUSE_LOCK bit to determine if PCH display is disabled

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v3)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v3)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 31 ++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h |  2 ++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e281298..283ace8 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1466,16 +1466,45 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
  *   - it's judged too laborious to fill n static structures with the limit
  *     when a simple if statement does the job,
  *   - run-time checks (eg read fuse/strap registers) are needed.
+ *
+ * This function needs to be called:
+ *   - after the MMIO has been setup as we are reading registers,
+ *   - after the PCH has been detected,
+ *   - before the first usage of the fields it can tweak.
  */
 static void intel_device_info_runtime_init(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_device_info *info;
 
-	info = (struct intel_device_info *)&to_i915(dev)->info;
+	info = (struct intel_device_info *)&dev_priv->info;
 
 	info->num_sprites = 1;
 	if (IS_VALLEYVIEW(dev))
 		info->num_sprites = 2;
+
+	if ((INTEL_INFO(dev)->gen == 7 || INTEL_INFO(dev)->gen == 8) &&
+	    !IS_VALLEYVIEW(dev)) {
+		u32 fuse_strap = I915_READ(FUSE_STRAP);
+		u32 sfuse_strap = I915_READ(SFUSE_STRAP);
+
+		/*
+		 * SFUSE_STRAP is supposed to have a bit signalling the display
+		 * is fused off. Unfortunately it seems that, at least in
+		 * certain cases, fused off display means that PCH display
+		 * reads don't land anywhere. In that case, we read 0s.
+		 *
+		 * On CPT/PPT, we can detect this case as SFUSE_STRAP_FUSE_LOCK
+		 * should be set when taking over after the firmware.
+		 */
+		if (fuse_strap & ILK_INTERNAL_DISPLAY_DISABLE ||
+		    sfuse_strap & SFUSE_STRAP_DISPLAY_DISABLED ||
+		    (dev_priv->pch_type == PCH_CPT &&
+		     !(sfuse_strap & SFUSE_STRAP_FUSE_LOCK))) {
+			DRM_INFO("Display fused off, disabling\n");
+			info->num_pipes = 0;
+		}
+	}
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 50968ef..aa5cfd7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5433,6 +5433,8 @@
 
 /* SFUSE_STRAP */
 #define SFUSE_STRAP			0xc2014
+#define  SFUSE_STRAP_FUSE_LOCK		(1<<13)
+#define  SFUSE_STRAP_DISPLAY_DISABLED	(1<<7)
 #define  SFUSE_STRAP_DDIB_DETECTED	(1<<2)
 #define  SFUSE_STRAP_DDIC_DETECTED	(1<<1)
 #define  SFUSE_STRAP_DDID_DETECTED	(1<<0)
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/8] drm/i915: Disable display when fused off

[Daniel Vetter]

On Fri, Feb 07, 2014 at 07:12:51PM +0000, Damien Lespiau wrote:
> FUSE_STRAP has a bit to inform us that the display has been fused off.
> Use it to setup the definitive number of pipes at run-time.
> 
> v2: actually tweak num_pipes, not num_planes
> v3: also tests SFUSE_STRAP bit 7
> v4: rebase on top of drm-nightly
>     use DRM_INFO() for the message telling display is fused off
>     try to read the FUSE_LOCK bit to determine if PCH display is disabled
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v3)
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v3)
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 31 ++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h |  2 ++
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index e281298..283ace8 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1466,16 +1466,45 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
>   *   - it's judged too laborious to fill n static structures with the limit
>   *     when a simple if statement does the job,
>   *   - run-time checks (eg read fuse/strap registers) are needed.
> + *
> + * This function needs to be called:
> + *   - after the MMIO has been setup as we are reading registers,
> + *   - after the PCH has been detected,
> + *   - before the first usage of the fields it can tweak.
>   */
>  static void intel_device_info_runtime_init(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_device_info *info;
>  
> -	info = (struct intel_device_info *)&to_i915(dev)->info;
> +	info = (struct intel_device_info *)&dev_priv->info;
>  
>  	info->num_sprites = 1;
>  	if (IS_VALLEYVIEW(dev))
>  		info->num_sprites = 2;
> +
> +	if ((INTEL_INFO(dev)->gen == 7 || INTEL_INFO(dev)->gen == 8) &&
> +	    !IS_VALLEYVIEW(dev)) {

I think we need a num_pipes > 0 check here, too. Our display hw guy
expressed surprise that we didn't hang the machine when reading pch
registers on a hsw platform, and Ben learned tha hard way that any such
access on ivb hard-hangs the box. Hence I think we need to retain the
current code's invariant that we never access the pch registers when
num_pipes is 0, like with one of the special pci matching rules.

I've merged the preceding patches to dinq meanwhile.
-Daniel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 5/8 v5] drm/i915: Disable display when fused off

[Damien Lespiau]

FUSE_STRAP has a bit to inform us that the display has been fused off.
Use it to setup the definitive number of pipes at run-time.

v2: actually tweak num_pipes, not num_planes
v3: also tests SFUSE_STRAP bit 7
v4: rebase on top of drm-nightly
    use DRM_INFO() for the message telling display is fused off
    try to read the FUSE_LOCK bit to determine if PCH display is disabled
v5: Don't read SFUSE_STRAP (register on the PCH) if num_pipes is already 0
    from the initial device info struct (to prevent hangs) (Daniel Vetter)

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v3)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v3)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 32 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h |  2 ++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e281298..033c943 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1466,16 +1466,46 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
  *   - it's judged too laborious to fill n static structures with the limit
  *     when a simple if statement does the job,
  *   - run-time checks (eg read fuse/strap registers) are needed.
+ *
+ * This function needs to be called:
+ *   - after the MMIO has been setup as we are reading registers,
+ *   - after the PCH has been detected,
+ *   - before the first usage of the fields it can tweak.
  */
 static void intel_device_info_runtime_init(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_device_info *info;
 
-	info = (struct intel_device_info *)&to_i915(dev)->info;
+	info = (struct intel_device_info *)&dev_priv->info;
 
 	info->num_sprites = 1;
 	if (IS_VALLEYVIEW(dev))
 		info->num_sprites = 2;
+
+	if (info->num_pipes > 0 &&
+	    (INTEL_INFO(dev)->gen == 7 || INTEL_INFO(dev)->gen == 8) &&
+	    !IS_VALLEYVIEW(dev)) {
+		u32 fuse_strap = I915_READ(FUSE_STRAP);
+		u32 sfuse_strap = I915_READ(SFUSE_STRAP);
+
+		/*
+		 * SFUSE_STRAP is supposed to have a bit signalling the display
+		 * is fused off. Unfortunately it seems that, at least in
+		 * certain cases, fused off display means that PCH display
+		 * reads don't land anywhere. In that case, we read 0s.
+		 *
+		 * On CPT/PPT, we can detect this case as SFUSE_STRAP_FUSE_LOCK
+		 * should be set when taking over after the firmware.
+		 */
+		if (fuse_strap & ILK_INTERNAL_DISPLAY_DISABLE ||
+		    sfuse_strap & SFUSE_STRAP_DISPLAY_DISABLED ||
+		    (dev_priv->pch_type == PCH_CPT &&
+		     !(sfuse_strap & SFUSE_STRAP_FUSE_LOCK))) {
+			DRM_INFO("Display fused off, disabling\n");
+			info->num_pipes = 0;
+		}
+	}
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d93f594..4846476 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5434,6 +5434,8 @@
 
 /* SFUSE_STRAP */
 #define SFUSE_STRAP			0xc2014
+#define  SFUSE_STRAP_FUSE_LOCK		(1<<13)
+#define  SFUSE_STRAP_DISPLAY_DISABLED	(1<<7)
 #define  SFUSE_STRAP_DDIB_DETECTED	(1<<2)
 #define  SFUSE_STRAP_DDIC_DETECTED	(1<<1)
 #define  SFUSE_STRAP_DDID_DETECTED	(1<<0)
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/8] drm/i915: Use I915_MAX_PIPES in the pipe/plane_to_crtc_mapping definitions

[Damien Lespiau]

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 36ea189..80ff7df 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1500,8 +1500,8 @@ typedef struct drm_i915_private {
 
 	struct sdvo_device_mapping sdvo_mappings[2];
 
-	struct drm_crtc *plane_to_crtc_mapping[3];
-	struct drm_crtc *pipe_to_crtc_mapping[3];
+	struct drm_crtc *plane_to_crtc_mapping[I915_MAX_PIPES];
+	struct drm_crtc *pipe_to_crtc_mapping[I915_MAX_PIPES];
 	wait_queue_head_t pending_flip_queue;
 
 #ifdef CONFIG_DEBUG_FS
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/8] drm/i915: Reorder i915_params fields to not create holes

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 80ff7df..b1e91c3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1944,18 +1944,19 @@ struct i915_params {
 	int vbt_sdvo_panel_type;
 	int enable_rc6;
 	int enable_fbc;
-	bool enable_hangcheck;
 	int enable_ppgtt;
 	int enable_psr;
 	unsigned int preliminary_hw_support;
 	int disable_power_well;
 	int enable_ips;
-	bool fastboot;
 	int enable_pc8;
 	int pc8_timeout;
+	int invert_brightness;
+	/* leave bools at the end to not create holes */
+	bool enable_hangcheck;
+	bool fastboot;
 	bool prefault_disable;
 	bool reset;
-	int invert_brightness;
 };
 extern struct i915_params i915 __read_mostly;
 
-- 
1.8.3.1

[PATCH 8/8] drm/i915: Provide a command line option to disable display

[Damien Lespiau]

If we can't actually determine at run-time we have a fused-off display,
provide at least an option to disable it.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c    | 3 ++-
 drivers/gpu/drm/i915/i915_drv.h    | 1 +
 drivers/gpu/drm/i915/i915_params.c | 4 ++++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 283ace8..493c9ea 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1497,7 +1497,8 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
 		 * On CPT/PPT, we can detect this case as SFUSE_STRAP_FUSE_LOCK
 		 * should be set when taking over after the firmware.
 		 */
-		if (fuse_strap & ILK_INTERNAL_DISPLAY_DISABLE ||
+		if (i915.disable_display ||
+		    fuse_strap & ILK_INTERNAL_DISPLAY_DISABLE ||
 		    sfuse_strap & SFUSE_STRAP_DISPLAY_DISABLED ||
 		    (dev_priv->pch_type == PCH_CPT &&
 		     !(sfuse_strap & SFUSE_STRAP_FUSE_LOCK))) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b1e91c3..47b207d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1957,6 +1957,7 @@ struct i915_params {
 	bool fastboot;
 	bool prefault_disable;
 	bool reset;
+	bool disable_display;
 };
 extern struct i915_params i915 __read_mostly;
 
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index c743057..3b48258 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -47,6 +47,7 @@ struct i915_params i915 __read_mostly = {
 	.prefault_disable = 0,
 	.reset = true,
 	.invert_brightness = 0,
+	.disable_display = 0,
 };
 
 module_param_named(modeset, i915.modeset, int, 0400);
@@ -153,3 +154,6 @@ MODULE_PARM_DESC(invert_brightness,
 	"report PCI device ID, subsystem vendor and subsystem device ID "
 	"to dri-devel@lists.freedesktop.org, if your machine needs it. "
 	"It will then be included in an upcoming module version.");
+
+module_param_named(disable_display, i915.disable_display, bool, 0600);
+MODULE_PARM_DESC(disable_display, "Disable display (default: false)");
-- 
1.8.3.1

Re: [PATCH 8/8] drm/i915: Provide a command line option to disable display

[Daniel Vetter]

On Fri, Feb 07, 2014 at 07:12:54PM +0000, Damien Lespiau wrote:
> If we can't actually determine at run-time we have a fused-off display,
> provide at least an option to disable it.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c    | 3 ++-
>  drivers/gpu/drm/i915/i915_drv.h    | 1 +
>  drivers/gpu/drm/i915/i915_params.c | 4 ++++
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 283ace8..493c9ea 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1497,7 +1497,8 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>  		 * On CPT/PPT, we can detect this case as SFUSE_STRAP_FUSE_LOCK
>  		 * should be set when taking over after the firmware.
>  		 */
> -		if (fuse_strap & ILK_INTERNAL_DISPLAY_DISABLE ||
> +		if (i915.disable_display ||

tbh I'd move this out into a separate top-level check - this option is a
bit hard to catch in the fuse checks here. Alos that way I could have
merged it already ;-)

Previous two patches are also merged, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 8/8 v2] drm/i915: Provide a command line option to disable display

[Damien Lespiau]

If we can't actually determine at run-time we have a fused-off display,
provide at least an option to disable it.

v2: Move the i915.disable_display test in a separate check
    (Daniel Vetter)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c    | 9 ++++++---
 drivers/gpu/drm/i915/i915_drv.h    | 1 +
 drivers/gpu/drm/i915/i915_params.c | 4 ++++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 033c943..1d65dbd 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1483,9 +1483,12 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
 	if (IS_VALLEYVIEW(dev))
 		info->num_sprites = 2;
 
-	if (info->num_pipes > 0 &&
-	    (INTEL_INFO(dev)->gen == 7 || INTEL_INFO(dev)->gen == 8) &&
-	    !IS_VALLEYVIEW(dev)) {
+	if (i915.disable_display) {
+		DRM_INFO("Display disabled (module parameter)\n");
+		info->num_pipes = 0;
+	} else if (info->num_pipes > 0 &&
+		   (INTEL_INFO(dev)->gen == 7 || INTEL_INFO(dev)->gen == 8) &&
+		   !IS_VALLEYVIEW(dev)) {
 		u32 fuse_strap = I915_READ(FUSE_STRAP);
 		u32 sfuse_strap = I915_READ(SFUSE_STRAP);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a77ff73..1369336 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1965,6 +1965,7 @@ struct i915_params {
 	bool fastboot;
 	bool prefault_disable;
 	bool reset;
+	bool disable_display;
 };
 extern struct i915_params i915 __read_mostly;
 
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index c743057..3b48258 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -47,6 +47,7 @@ struct i915_params i915 __read_mostly = {
 	.prefault_disable = 0,
 	.reset = true,
 	.invert_brightness = 0,
+	.disable_display = 0,
 };
 
 module_param_named(modeset, i915.modeset, int, 0400);
@@ -153,3 +154,6 @@ MODULE_PARM_DESC(invert_brightness,
 	"report PCI device ID, subsystem vendor and subsystem device ID "
 	"to dri-devel@lists.freedesktop.org, if your machine needs it. "
 	"It will then be included in an upcoming module version.");
+
+module_param_named(disable_display, i915.disable_display, bool, 0600);
+MODULE_PARM_DESC(disable_display, "Disable display (default: false)");
-- 
1.8.3.1

Re: [PATCH 8/8 v2] drm/i915: Provide a command line option to disable display

[Daniel Vetter]

On Mon, Feb 10, 2014 at 05:20:55PM +0000, Damien Lespiau wrote:
> If we can't actually determine at run-time we have a fused-off display,
> provide at least an option to disable it.
> 
> v2: Move the i915.disable_display test in a separate check
>     (Daniel Vetter)
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Both resends merged to dinq, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch