All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2
@ 2013-09-04 15:25 ville.syrjala
  2013-09-04 15:25 ` [PATCH v2 01/14] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode ville.syrjala
                   ` (13 more replies)
  0 siblings, 14 replies; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

Same as before with a few minor fixes, and rebased on top of the
"drm/i915: adjusted_mode.clock vs. port_clock v2" series.

I also included the cursor fixes since the second one depends on
pipe_src_{w,h} being available.

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

* [PATCH v2 01/14] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 14:27   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 02/14] drm/i915: Use adjusted_mode->clock in lpt_program_iclkip ville.syrjala
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

i9xx_set_pipeconf() attempts to get the current pixel clock from
requested_mode. requested_mode.clock may be totally bogus, so the
clock should come from adjusted_mode.

v2: Dropped the intel_compute_config() hunk due to killing of the
    INTEL_FDI_FREQ check

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a4fe594..2c450fe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4786,7 +4786,7 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 		 * XXX: No double-wide on 915GM pipe B. Is that the only reason for the
 		 * pipe == 0 check?
 		 */
-		if (intel_crtc->config.requested_mode.clock >
+		if (intel_crtc->config.adjusted_mode.clock >
 		    dev_priv->display.get_display_clock_speed(dev) * 9 / 10)
 			pipeconf |= PIPECONF_DOUBLE_WIDE;
 	}
-- 
1.8.1.5

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

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

* [PATCH 02/14] drm/i915: Use adjusted_mode->clock in lpt_program_iclkip
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
  2013-09-04 15:25 ` [PATCH v2 01/14] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 14:41   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 03/14] drm/i915: Use adjusted_mode in HDMI 12bpc clock check ville.syrjala
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

lpt_program_iclkip() wants to know the pixel clock. It should get that
information from adjusted_mode, not crtc->mode.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2c450fe..fd67758 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2875,6 +2875,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
 	u32 divsel, phaseinc, auxdiv, phasedir = 0;
 	u32 temp;
 
@@ -2892,13 +2893,13 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 			SBI_ICLK);
 
 	/* 20MHz is a corner case which is out of range for the 7-bit divisor */
-	if (crtc->mode.clock == 20000) {
+	if (clock == 20000) {
 		auxdiv = 1;
 		divsel = 0x41;
 		phaseinc = 0x20;
 	} else {
 		/* The iCLK virtual clock root frequency is in MHz,
-		 * but the crtc->mode.clock in in KHz. To get the divisors,
+		 * but the adjusted_mode->clock in in KHz. To get the divisors,
 		 * it is necessary to divide one by another, so we
 		 * convert the virtual clock precision to KHz here for higher
 		 * precision.
@@ -2907,7 +2908,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 		u32 iclk_pi_range = 64;
 		u32 desired_divisor, msb_divisor_value, pi_value;
 
-		desired_divisor = (iclk_virtual_root_freq / crtc->mode.clock);
+		desired_divisor = (iclk_virtual_root_freq / clock);
 		msb_divisor_value = desired_divisor / iclk_pi_range;
 		pi_value = desired_divisor % iclk_pi_range;
 
@@ -2923,7 +2924,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
 		~SBI_SSCDIVINTPHASE_INCVAL_MASK);
 
 	DRM_DEBUG_KMS("iCLKIP clock: found settings for %dKHz refresh rate: auxdiv=%x, divsel=%x, phasedir=%x, phaseinc=%x\n",
-			crtc->mode.clock,
+			clock,
 			auxdiv,
 			divsel,
 			phasedir,
-- 
1.8.1.5

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

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

* [PATCH 03/14] drm/i915: Use adjusted_mode in HDMI 12bpc clock check
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
  2013-09-04 15:25 ` [PATCH v2 01/14] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode ville.syrjala
  2013-09-04 15:25 ` [PATCH 02/14] drm/i915: Use adjusted_mode->clock in lpt_program_iclkip ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 14:42   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 04/14] drm/i915: Use adjusted_mode in intel_update_fbc() ville.syrjala
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The pixel clock should come from adjusted_mode not requested_mode.
In this case the two should be the same as we don't currently
overwrite the clock in the case of HDMI. But let's make the code
safe against such things happening in the future.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 3d833a3..6e87b83 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -865,7 +865,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
-	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
+	int clock_12bpc = pipe_config->adjusted_mode.clock * 3 / 2;
 	int portclock_limit = hdmi_portclock_limit(intel_hdmi);
 	int desired_bpp;
 
-- 
1.8.1.5

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

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

* [PATCH 04/14] drm/i915: Use adjusted_mode in intel_update_fbc()
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (2 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH 03/14] drm/i915: Use adjusted_mode in HDMI 12bpc clock check ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 14:45   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 05/14] drm/i915: Use adjusted_mode appropriately when computing watermarks ville.syrjala
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Check the mode flags from the adjusted_mode, not user requested mode.
The hdisplay/vdisplay check actually checkes the primary plane size,
so those still need to come from the user requested mode.

Extract both modes from pipe config instead of the drm_crtc.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0c115cc..af1f4de 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -458,6 +458,8 @@ void intel_update_fbc(struct drm_device *dev)
 	struct drm_framebuffer *fb;
 	struct intel_framebuffer *intel_fb;
 	struct drm_i915_gem_object *obj;
+	const struct drm_display_mode *mode;
+	const struct drm_display_mode *adjusted_mode;
 	unsigned int max_hdisplay, max_vdisplay;
 
 	if (!I915_HAS_FBC(dev)) {
@@ -502,6 +504,8 @@ void intel_update_fbc(struct drm_device *dev)
 	fb = crtc->fb;
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
+	mode = &intel_crtc->config.requested_mode;
+	adjusted_mode = &intel_crtc->config.adjusted_mode;
 
 	if (i915_enable_fbc < 0 &&
 	    INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev)) {
@@ -514,8 +518,8 @@ void intel_update_fbc(struct drm_device *dev)
 			DRM_DEBUG_KMS("fbc disabled per module param\n");
 		goto out_disable;
 	}
-	if ((crtc->mode.flags & DRM_MODE_FLAG_INTERLACE) ||
-	    (crtc->mode.flags & DRM_MODE_FLAG_DBLSCAN)) {
+	if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
+	    (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
 		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
 			DRM_DEBUG_KMS("mode incompatible with compression, "
 				      "disabling\n");
@@ -529,8 +533,8 @@ void intel_update_fbc(struct drm_device *dev)
 		max_hdisplay = 2048;
 		max_vdisplay = 1536;
 	}
-	if ((crtc->mode.hdisplay > max_hdisplay) ||
-	    (crtc->mode.vdisplay > max_vdisplay)) {
+	if ((mode->hdisplay > max_hdisplay) ||
+	    (mode->vdisplay > max_vdisplay)) {
 		if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
 			DRM_DEBUG_KMS("mode too large for compression, disabling\n");
 		goto out_disable;
-- 
1.8.1.5

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

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

* [PATCH 05/14] drm/i915: Use adjusted_mode appropriately when computing watermarks
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (3 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH 04/14] drm/i915: Use adjusted_mode in intel_update_fbc() ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 15:00   ` Damien Lespiau
  2013-09-10 16:08   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 06/14] drm/i915: Check the clock from adjusted mode in intel_crtc_active() ville.syrjala
                   ` (8 subsequent siblings)
  13 siblings, 2 replies; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently most of the watermark code looks at crtc->mode which is the
user requested mode. The only piece of information there that is
relevant is hdisplay, the rest must come from adjusted_mode. Convert
all of the code to use requested_mode and adjusted_mode from
pipe config appropriately.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 55 ++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index af1f4de..48d93d3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1109,7 +1109,7 @@ static void pineview_update_wm(struct drm_device *dev)
 
 	crtc = single_enabled_crtc(dev);
 	if (crtc) {
-		int clock = crtc->mode.clock;
+		int clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
 		int pixel_size = crtc->fb->bits_per_pixel / 8;
 
 		/* Display SR */
@@ -1170,6 +1170,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
 			    int *cursor_wm)
 {
 	struct drm_crtc *crtc;
+	const struct drm_display_mode *adjusted_mode;
 	int htotal, hdisplay, clock, pixel_size;
 	int line_time_us, line_count;
 	int entries, tlb_miss;
@@ -1181,9 +1182,10 @@ static bool g4x_compute_wm0(struct drm_device *dev,
 		return false;
 	}
 
-	htotal = crtc->mode.htotal;
-	hdisplay = crtc->mode.hdisplay;
-	clock = crtc->mode.clock;
+	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
+	clock = adjusted_mode->clock;
+	htotal = adjusted_mode->htotal;
+	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
 	pixel_size = crtc->fb->bits_per_pixel / 8;
 
 	/* Use the small buffer method to calculate plane watermark */
@@ -1254,6 +1256,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
 			     int *display_wm, int *cursor_wm)
 {
 	struct drm_crtc *crtc;
+	const struct drm_display_mode *adjusted_mode;
 	int hdisplay, htotal, pixel_size, clock;
 	unsigned long line_time_us;
 	int line_count, line_size;
@@ -1266,9 +1269,10 @@ static bool g4x_compute_srwm(struct drm_device *dev,
 	}
 
 	crtc = intel_get_crtc_for_plane(dev, plane);
-	hdisplay = crtc->mode.hdisplay;
-	htotal = crtc->mode.htotal;
-	clock = crtc->mode.clock;
+	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
+	clock = adjusted_mode->clock;
+	htotal = adjusted_mode->htotal;
+	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
 	pixel_size = crtc->fb->bits_per_pixel / 8;
 
 	line_time_us = (htotal * 1000) / clock;
@@ -1307,7 +1311,7 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
 	if (!intel_crtc_active(crtc))
 		return false;
 
-	clock = crtc->mode.clock;	/* VESA DOT Clock */
+	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
 	pixel_size = crtc->fb->bits_per_pixel / 8;	/* BPP */
 
 	entries = (clock / 1000) * pixel_size;
@@ -1492,9 +1496,11 @@ static void i965_update_wm(struct drm_device *dev)
 	if (crtc) {
 		/* self-refresh has much higher latency */
 		static const int sr_latency_ns = 12000;
-		int clock = crtc->mode.clock;
-		int htotal = crtc->mode.htotal;
-		int hdisplay = crtc->mode.hdisplay;
+		const struct drm_display_mode *adjusted_mode =
+			&to_intel_crtc(crtc)->config.adjusted_mode;
+		int clock = adjusted_mode->clock;
+		int htotal = adjusted_mode->htotal;
+		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
 		int pixel_size = crtc->fb->bits_per_pixel / 8;
 		unsigned long line_time_us;
 		int entries;
@@ -1570,7 +1576,7 @@ static void i9xx_update_wm(struct drm_device *dev)
 		if (IS_GEN2(dev))
 			cpp = 4;
 
-		planea_wm = intel_calculate_wm(crtc->mode.clock,
+		planea_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
 					       wm_info, fifo_size, cpp,
 					       latency_ns);
 		enabled = crtc;
@@ -1584,7 +1590,7 @@ static void i9xx_update_wm(struct drm_device *dev)
 		if (IS_GEN2(dev))
 			cpp = 4;
 
-		planeb_wm = intel_calculate_wm(crtc->mode.clock,
+		planeb_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
 					       wm_info, fifo_size, cpp,
 					       latency_ns);
 		if (enabled == NULL)
@@ -1611,9 +1617,11 @@ static void i9xx_update_wm(struct drm_device *dev)
 	if (HAS_FW_BLC(dev) && enabled) {
 		/* self-refresh has much higher latency */
 		static const int sr_latency_ns = 6000;
-		int clock = enabled->mode.clock;
-		int htotal = enabled->mode.htotal;
-		int hdisplay = enabled->mode.hdisplay;
+		const struct drm_display_mode *adjusted_mode =
+			&to_intel_crtc(enabled)->config.adjusted_mode;
+		int clock = adjusted_mode->clock;
+		int htotal = adjusted_mode->htotal;
+		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
 		int pixel_size = enabled->fb->bits_per_pixel / 8;
 		unsigned long line_time_us;
 		int entries;
@@ -1673,7 +1681,8 @@ static void i830_update_wm(struct drm_device *dev)
 	if (crtc == NULL)
 		return;
 
-	planea_wm = intel_calculate_wm(crtc->mode.clock, &i830_wm_info,
+	planea_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
+				       &i830_wm_info,
 				       dev_priv->display.get_fifo_size(dev, 0),
 				       4, latency_ns);
 	fwater_lo = I915_READ(FW_BLC) & ~0xfff;
@@ -1745,6 +1754,7 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
 				  int *fbc_wm, int *display_wm, int *cursor_wm)
 {
 	struct drm_crtc *crtc;
+	const struct drm_display_mode *adjusted_mode;
 	unsigned long line_time_us;
 	int hdisplay, htotal, pixel_size, clock;
 	int line_count, line_size;
@@ -1757,9 +1767,10 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
 	}
 
 	crtc = intel_get_crtc_for_plane(dev, plane);
-	hdisplay = crtc->mode.hdisplay;
-	htotal = crtc->mode.htotal;
-	clock = crtc->mode.clock;
+	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
+	clock = adjusted_mode->clock;
+	htotal = adjusted_mode->htotal;
+	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
 	pixel_size = crtc->fb->bits_per_pixel / 8;
 
 	line_time_us = (htotal * 1000) / clock;
@@ -2902,7 +2913,7 @@ sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
 		return false;
 	}
 
-	clock = crtc->mode.clock;
+	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
 
 	/* Use the small buffer method to calculate the sprite watermark */
 	entries = ((clock * pixel_size / 1000) * display_latency_ns) / 1000;
@@ -2937,7 +2948,7 @@ sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
 	}
 
 	crtc = intel_get_crtc_for_plane(dev, plane);
-	clock = crtc->mode.clock;
+	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
 	if (!clock) {
 		*sprite_wm = 0;
 		return false;
-- 
1.8.1.5

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

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

* [PATCH 06/14] drm/i915: Check the clock from adjusted mode in intel_crtc_active()
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (4 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH 05/14] drm/i915: Use adjusted_mode appropriately when computing watermarks ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 15:00   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 07/14] drm/i915: Use adjusted_mode when checking conditions for PSR ville.syrjala
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The clock in crtc->mode doesn't necessarily mean anything. Let's look
at the clock in adjusted_mode instead.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 48d93d3..397628b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -45,10 +45,13 @@
 
 static bool intel_crtc_active(struct drm_crtc *crtc)
 {
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
 	/* Be paranoid as we can arrive here with only partial
 	 * state retrieved from the hardware during setup.
 	 */
-	return to_intel_crtc(crtc)->active && crtc->fb && crtc->mode.clock;
+	return intel_crtc->active && crtc->fb &&
+		intel_crtc->config.adjusted_mode.clock;
 }
 
 static void i8xx_disable_fbc(struct drm_device *dev)
-- 
1.8.1.5

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

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

* [PATCH 07/14] drm/i915: Use adjusted_mode when checking conditions for PSR
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (5 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH 06/14] drm/i915: Check the clock from adjusted mode in intel_crtc_active() ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 16:42   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH v2 08/14] drm/i915: Make intel_crtc_active() available outside intel_pm.c ville.syrjala
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel_edp_psr_match_conditions() currently looks at crtc->mode
when it really needs to look at adjusted_mode. Fix it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c192dbb..3f0dd65 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1570,7 +1570,8 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 	}
 
 	intel_crtc = to_intel_crtc(crtc);
-	if (!intel_crtc->active || !crtc->fb || !crtc->mode.clock) {
+	if (!intel_crtc->active || !crtc->fb ||
+	    !intel_crtc->config.adjusted_mode.clock) {
 		DRM_DEBUG_KMS("crtc not active for PSR\n");
 		dev_priv->no_psr_reason = PSR_CRTC_NOT_ACTIVE;
 		return false;
@@ -1597,7 +1598,7 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 		return false;
 	}
 
-	if (crtc->mode.flags & DRM_MODE_FLAG_INTERLACE) {
+	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
 		dev_priv->no_psr_reason = PSR_INTERLACED_ENABLED;
 		return false;
-- 
1.8.1.5

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

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

* [PATCH v2 08/14] drm/i915: Make intel_crtc_active() available outside intel_pm.c
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (6 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH 07/14] drm/i915: Use adjusted_mode when checking conditions for PSR ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 16:43   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 09/14] drm/i915: Use pipe config in sprite code ville.syrjala
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Move intel_crtc_active() to intel_display.c and make it available
elsewhere as well.

intel_edp_psr_match_conditions() already has one open coded copy,
so replace that one with a call to intel_crtc_active().

v2: Copy paste a big comment from danvet's mail explaining
    when we can ditch the extra checks

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c      |  3 +--
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_pm.c      | 11 -----------
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fd67758..71c7763 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -733,6 +733,23 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 	return true;
 }
 
+bool intel_crtc_active(struct drm_crtc *crtc)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	/* Be paranoid as we can arrive here with only partial
+	 * state retrieved from the hardware during setup.
+	 *
+	 * We can ditch the adjusted_mode.clock check as soon
+	 * as Haswell has gained clock readout/fastboot support.
+	 *
+	 * We can ditch the crtc->fb check as soon as we can
+	 * properly reconstruct framebuffers.
+	 */
+	return intel_crtc->active && crtc->fb &&
+		intel_crtc->config.adjusted_mode.clock;
+}
+
 enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
 					     enum pipe pipe)
 {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3f0dd65..87b03b2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1570,8 +1570,7 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 	}
 
 	intel_crtc = to_intel_crtc(crtc);
-	if (!intel_crtc->active || !crtc->fb ||
-	    !intel_crtc->config.adjusted_mode.clock) {
+	if (!intel_crtc_active(crtc)) {
 		DRM_DEBUG_KMS("crtc not active for PSR\n");
 		dev_priv->no_psr_reason = PSR_CRTC_NOT_ACTIVE;
 		return false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dbf04be..59f0ec7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -801,5 +801,6 @@ extern void hsw_pc8_disable_interrupts(struct drm_device *dev);
 extern void hsw_pc8_restore_interrupts(struct drm_device *dev);
 extern void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
 extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
+extern bool intel_crtc_active(struct drm_crtc *crtc);
 
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 397628b..3ba412c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -43,17 +43,6 @@
  * i915.i915_enable_fbc parameter
  */
 
-static bool intel_crtc_active(struct drm_crtc *crtc)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	/* Be paranoid as we can arrive here with only partial
-	 * state retrieved from the hardware during setup.
-	 */
-	return intel_crtc->active && crtc->fb &&
-		intel_crtc->config.adjusted_mode.clock;
-}
-
 static void i8xx_disable_fbc(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
1.8.1.5

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

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

* [PATCH 09/14] drm/i915: Use pipe config in sprite code
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (7 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH v2 08/14] drm/i915: Make intel_crtc_active() available outside intel_pm.c ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 16:44   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 10/14] drm/i915: Use adjusted_mode in DSI PLL calculations ville.syrjala
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather than dig up the pipe source size from crtc->mode, use
intel_crtc->config.requested_mode.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index ad6ec4b..753cef3 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -652,8 +652,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		.y2 = crtc_y + crtc_h,
 	};
 	const struct drm_rect clip = {
-		.x2 = crtc->mode.hdisplay,
-		.y2 = crtc->mode.vdisplay,
+		.x2 = intel_crtc->config.requested_mode.hdisplay,
+		.y2 = intel_crtc->config.requested_mode.vdisplay,
 	};
 
 	intel_fb = to_intel_framebuffer(fb);
-- 
1.8.1.5

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

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

* [PATCH 10/14] drm/i915: Use adjusted_mode in DSI PLL calculations
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (8 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH 09/14] drm/i915: Use pipe config in sprite code ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 16:45   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH v2 11/14] drm/i915: Add explicit pipe src size to pipe config ville.syrjala
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

adjusted_mode contains our real timings, not requested_mode. Use the
correct thing in DSI PLL code.

Also constify adjusted_mode since we don't change it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 582f626..44279b2 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -50,7 +50,7 @@ static const u32 lfsr_converts[] = {
 	71, 35							/* 91 - 92 */
 };
 
-static u32 dsi_rr_formula(struct drm_display_mode *mode,
+static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 			  int pixel_format, int video_mode_format,
 			  int lane_count, bool eotp)
 {
@@ -245,7 +245,7 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
-	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
+	const struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	int ret;
 	struct dsi_mnp dsi_mnp;
-- 
1.8.1.5

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

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

* [PATCH v2 11/14] drm/i915: Add explicit pipe src size to pipe config
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (9 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH 10/14] drm/i915: Use adjusted_mode in DSI PLL calculations ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 17:02   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 12/14] drm/i915: Document the inteded use of requested_mode ville.syrjala
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather that mess about with hdisplay/vdisplay from requested_mode, add
explicit pipe src size information to pipe config.

Now requested_mode is only really relevant for dvo/sdvo output timings.
For everything else either adjusted_mode or pipe src size should be
used.

In many places where we end up using pipe source size, we should
actually use the primary plane size, but we don't currently store
that information explicitly. As long as we treat primaries as full
screen only, we can get away with this. Eventually when we move
primaries over to drm_plane, we need to fix it all up.

v2: Add a comment to explain what pipe_src_{w,h} are
    Add a note about primary planes to commit message

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |  6 ++++
 drivers/gpu/drm/i915/intel_panel.c   | 56 +++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_pm.c      | 33 ++++++++++-----------
 drivers/gpu/drm/i915/intel_sprite.c  |  4 +--
 5 files changed, 67 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 71c7763..8d64d54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4673,7 +4673,6 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 	enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
 	struct drm_display_mode *adjusted_mode =
 		&intel_crtc->config.adjusted_mode;
-	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	uint32_t vsyncshift, crtc_vtotal, crtc_vblank_end;
 
 	/* We need to be careful not to changed the adjusted mode, for otherwise
@@ -4726,7 +4725,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 	 * always be the user's requested size.
 	 */
 	I915_WRITE(PIPESRC(pipe),
-		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
+		   ((intel_crtc->config.pipe_src_w - 1) << 16) |
+		   (intel_crtc->config.pipe_src_h - 1));
 }
 
 static void intel_get_pipe_timings(struct intel_crtc *crtc,
@@ -4764,8 +4764,11 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
 	}
 
 	tmp = I915_READ(PIPESRC(crtc->pipe));
-	pipe_config->requested_mode.vdisplay = (tmp & 0xffff) + 1;
-	pipe_config->requested_mode.hdisplay = ((tmp >> 16) & 0xffff) + 1;
+	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
+	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
+
+	pipe_config->requested_mode.vdisplay = pipe_config->pipe_src_h;
+	pipe_config->requested_mode.hdisplay = pipe_config->pipe_src_w;
 }
 
 static void intel_crtc_mode_from_pipe_config(struct intel_crtc *intel_crtc,
@@ -4861,7 +4864,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
 	int refclk, num_connectors = 0;
@@ -4960,8 +4962,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	 * which should always be the user's requested size.
 	 */
 	I915_WRITE(DSPSIZE(plane),
-		   ((mode->vdisplay - 1) << 16) |
-		   (mode->hdisplay - 1));
+		   ((intel_crtc->config.pipe_src_h - 1) << 16) |
+		   (intel_crtc->config.pipe_src_w - 1));
 	I915_WRITE(DSPPOS(plane), 0);
 
 	i9xx_set_pipeconf(intel_crtc);
@@ -8281,6 +8283,8 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	DRM_DEBUG_KMS("adjusted mode:\n");
 	drm_mode_debug_printmodeline(&pipe_config->adjusted_mode);
 	DRM_DEBUG_KMS("port clock: %d\n", pipe_config->port_clock);
+	DRM_DEBUG_KMS("pipe src size: %dx%d\n",
+		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
 	DRM_DEBUG_KMS("gmch pfit: control: 0x%08x, ratios: 0x%08x, lvds border: 0x%08x\n",
 		      pipe_config->gmch_pfit.control,
 		      pipe_config->gmch_pfit.pgm_ratios,
@@ -8332,6 +8336,10 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 
 	drm_mode_copy(&pipe_config->adjusted_mode, mode);
 	drm_mode_copy(&pipe_config->requested_mode, mode);
+
+	pipe_config->pipe_src_w = mode->hdisplay;
+	pipe_config->pipe_src_h = mode->vdisplay;
+
 	pipe_config->cpu_transcoder =
 		(enum transcoder) to_intel_crtc(crtc)->pipe;
 	pipe_config->shared_dpll = DPLL_ID_PRIVATE;
@@ -8681,8 +8689,8 @@ intel_pipe_config_compare(struct drm_device *dev,
 				      DRM_MODE_FLAG_NVSYNC);
 	}
 
-	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
-	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
+	PIPE_CONF_CHECK_I(pipe_src_w);
+	PIPE_CONF_CHECK_I(pipe_src_h);
 
 	PIPE_CONF_CHECK_I(gmch_pfit.control);
 	/* pfit ratios are autocomputed by the hw on gen4+ */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 59f0ec7..9d30909 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -215,6 +215,12 @@ struct intel_crtc_config {
 	/* Actual pipe timings ie. what we program into the pipe timing
 	 * registers. adjusted_mode.clock is the pipe pixel clock. */
 	struct drm_display_mode adjusted_mode;
+
+	/* Pipe source size (ie. panel fitter input size)
+	 * All planes will be positioned inside this space,
+	 * and get clipped at the edges. */
+	int pipe_src_w, pipe_src_h;
+
 	/* Whether to set up the PCH/FDI. Note that we never allow sharing
 	 * between pch encoders and cpu encoders. */
 	bool has_pch_encoder;
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 42114ec..c9dba46 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -50,23 +50,22 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 			struct intel_crtc_config *pipe_config,
 			int fitting_mode)
 {
-	struct drm_display_mode *mode, *adjusted_mode;
+	struct drm_display_mode *adjusted_mode;
 	int x, y, width, height;
 
-	mode = &pipe_config->requested_mode;
 	adjusted_mode = &pipe_config->adjusted_mode;
 
 	x = y = width = height = 0;
 
 	/* Native modes don't need fitting */
-	if (adjusted_mode->hdisplay == mode->hdisplay &&
-	    adjusted_mode->vdisplay == mode->vdisplay)
+	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
+	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
 		goto done;
 
 	switch (fitting_mode) {
 	case DRM_MODE_SCALE_CENTER:
-		width = mode->hdisplay;
-		height = mode->vdisplay;
+		width = pipe_config->pipe_src_w;
+		height = pipe_config->pipe_src_h;
 		x = (adjusted_mode->hdisplay - width + 1)/2;
 		y = (adjusted_mode->vdisplay - height + 1)/2;
 		break;
@@ -74,17 +73,17 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 	case DRM_MODE_SCALE_ASPECT:
 		/* Scale but preserve the aspect ratio */
 		{
-			u32 scaled_width = adjusted_mode->hdisplay * mode->vdisplay;
-			u32 scaled_height = mode->hdisplay * adjusted_mode->vdisplay;
+			u32 scaled_width = adjusted_mode->hdisplay * pipe_config->pipe_src_h;
+			u32 scaled_height = pipe_config->pipe_src_w * adjusted_mode->vdisplay;
 			if (scaled_width > scaled_height) { /* pillar */
-				width = scaled_height / mode->vdisplay;
+				width = scaled_height / pipe_config->pipe_src_h;
 				if (width & 1)
 					width++;
 				x = (adjusted_mode->hdisplay - width + 1) / 2;
 				y = 0;
 				height = adjusted_mode->vdisplay;
 			} else if (scaled_width < scaled_height) { /* letter */
-				height = scaled_width / mode->hdisplay;
+				height = scaled_width / pipe_config->pipe_src_w;
 				if (height & 1)
 				    height++;
 				y = (adjusted_mode->vdisplay - height + 1) / 2;
@@ -176,14 +175,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
-	struct drm_display_mode *mode, *adjusted_mode;
+	struct drm_display_mode *adjusted_mode;
 
-	mode = &pipe_config->requested_mode;
 	adjusted_mode = &pipe_config->adjusted_mode;
 
 	/* Native modes don't need fitting */
-	if (adjusted_mode->hdisplay == mode->hdisplay &&
-	    adjusted_mode->vdisplay == mode->vdisplay)
+	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
+	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
 		goto out;
 
 	switch (fitting_mode) {
@@ -192,16 +190,16 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 		 * For centered modes, we have to calculate border widths &
 		 * heights and modify the values programmed into the CRTC.
 		 */
-		centre_horizontally(adjusted_mode, mode->hdisplay);
-		centre_vertically(adjusted_mode, mode->vdisplay);
+		centre_horizontally(adjusted_mode, pipe_config->pipe_src_w);
+		centre_vertically(adjusted_mode, pipe_config->pipe_src_h);
 		border = LVDS_BORDER_ENABLE;
 		break;
 	case DRM_MODE_SCALE_ASPECT:
 		/* Scale but preserve the aspect ratio */
 		if (INTEL_INFO(dev)->gen >= 4) {
 			u32 scaled_width = adjusted_mode->hdisplay *
-				mode->vdisplay;
-			u32 scaled_height = mode->hdisplay *
+				pipe_config->pipe_src_h;
+			u32 scaled_height = pipe_config->pipe_src_w *
 				adjusted_mode->vdisplay;
 
 			/* 965+ is easy, it does everything in hw */
@@ -211,12 +209,12 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			else if (scaled_width < scaled_height)
 				pfit_control |= PFIT_ENABLE |
 					PFIT_SCALING_LETTER;
-			else if (adjusted_mode->hdisplay != mode->hdisplay)
+			else if (adjusted_mode->hdisplay != pipe_config->pipe_src_w)
 				pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
 		} else {
 			u32 scaled_width = adjusted_mode->hdisplay *
-				mode->vdisplay;
-			u32 scaled_height = mode->hdisplay *
+				pipe_config->pipe_src_h;
+			u32 scaled_height = pipe_config->pipe_src_w *
 				adjusted_mode->vdisplay;
 			/*
 			 * For earlier chips we have to calculate the scaling
@@ -226,11 +224,11 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			if (scaled_width > scaled_height) { /* pillar */
 				centre_horizontally(adjusted_mode,
 						    scaled_height /
-						    mode->vdisplay);
+						    pipe_config->pipe_src_h);
 
 				border = LVDS_BORDER_ENABLE;
-				if (mode->vdisplay != adjusted_mode->vdisplay) {
-					u32 bits = panel_fitter_scaling(mode->vdisplay, adjusted_mode->vdisplay);
+				if (pipe_config->pipe_src_h != adjusted_mode->vdisplay) {
+					u32 bits = panel_fitter_scaling(pipe_config->pipe_src_h, adjusted_mode->vdisplay);
 					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
 							    bits << PFIT_VERT_SCALE_SHIFT);
 					pfit_control |= (PFIT_ENABLE |
@@ -240,11 +238,11 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 			} else if (scaled_width < scaled_height) { /* letter */
 				centre_vertically(adjusted_mode,
 						  scaled_width /
-						  mode->hdisplay);
+						  pipe_config->pipe_src_w);
 
 				border = LVDS_BORDER_ENABLE;
-				if (mode->hdisplay != adjusted_mode->hdisplay) {
-					u32 bits = panel_fitter_scaling(mode->hdisplay, adjusted_mode->hdisplay);
+				if (pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
+					u32 bits = panel_fitter_scaling(pipe_config->pipe_src_w, adjusted_mode->hdisplay);
 					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
 							    bits << PFIT_VERT_SCALE_SHIFT);
 					pfit_control |= (PFIT_ENABLE |
@@ -265,8 +263,8 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 		 * Full scaling, even if it changes the aspect ratio.
 		 * Fortunately this is all done for us in hw.
 		 */
-		if (mode->vdisplay != adjusted_mode->vdisplay ||
-		    mode->hdisplay != adjusted_mode->hdisplay) {
+		if (pipe_config->pipe_src_h != adjusted_mode->vdisplay ||
+		    pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
 			pfit_control |= PFIT_ENABLE;
 			if (INTEL_INFO(dev)->gen >= 4)
 				pfit_control |= PFIT_SCALING_AUTO;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3ba412c..3823d63 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -450,9 +450,8 @@ void intel_update_fbc(struct drm_device *dev)
 	struct drm_framebuffer *fb;
 	struct intel_framebuffer *intel_fb;
 	struct drm_i915_gem_object *obj;
-	const struct drm_display_mode *mode;
 	const struct drm_display_mode *adjusted_mode;
-	unsigned int max_hdisplay, max_vdisplay;
+	unsigned int max_width, max_height;
 
 	if (!I915_HAS_FBC(dev)) {
 		set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
@@ -496,7 +495,6 @@ void intel_update_fbc(struct drm_device *dev)
 	fb = crtc->fb;
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
-	mode = &intel_crtc->config.requested_mode;
 	adjusted_mode = &intel_crtc->config.adjusted_mode;
 
 	if (i915_enable_fbc < 0 &&
@@ -519,14 +517,14 @@ void intel_update_fbc(struct drm_device *dev)
 	}
 
 	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
-		max_hdisplay = 4096;
-		max_vdisplay = 2048;
+		max_width = 4096;
+		max_height = 2048;
 	} else {
-		max_hdisplay = 2048;
-		max_vdisplay = 1536;
+		max_width = 2048;
+		max_height = 1536;
 	}
-	if ((mode->hdisplay > max_hdisplay) ||
-	    (mode->vdisplay > max_vdisplay)) {
+	if (intel_crtc->config.pipe_src_w > max_width ||
+	    intel_crtc->config.pipe_src_h > max_height) {
 		if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
 			DRM_DEBUG_KMS("mode too large for compression, disabling\n");
 		goto out_disable;
@@ -1177,7 +1175,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
 	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
 	clock = adjusted_mode->clock;
 	htotal = adjusted_mode->htotal;
-	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
+	hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
 	pixel_size = crtc->fb->bits_per_pixel / 8;
 
 	/* Use the small buffer method to calculate plane watermark */
@@ -1264,7 +1262,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
 	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
 	clock = adjusted_mode->clock;
 	htotal = adjusted_mode->htotal;
-	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
+	hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
 	pixel_size = crtc->fb->bits_per_pixel / 8;
 
 	line_time_us = (htotal * 1000) / clock;
@@ -1492,7 +1490,7 @@ static void i965_update_wm(struct drm_device *dev)
 			&to_intel_crtc(crtc)->config.adjusted_mode;
 		int clock = adjusted_mode->clock;
 		int htotal = adjusted_mode->htotal;
-		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
+		int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
 		int pixel_size = crtc->fb->bits_per_pixel / 8;
 		unsigned long line_time_us;
 		int entries;
@@ -1613,7 +1611,7 @@ static void i9xx_update_wm(struct drm_device *dev)
 			&to_intel_crtc(enabled)->config.adjusted_mode;
 		int clock = adjusted_mode->clock;
 		int htotal = adjusted_mode->htotal;
-		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
+		int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
 		int pixel_size = enabled->fb->bits_per_pixel / 8;
 		unsigned long line_time_us;
 		int entries;
@@ -1762,7 +1760,7 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
 	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
 	clock = adjusted_mode->clock;
 	htotal = adjusted_mode->htotal;
-	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
+	hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
 	pixel_size = crtc->fb->bits_per_pixel / 8;
 
 	line_time_us = (htotal * 1000) / clock;
@@ -2114,8 +2112,8 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
 	if (pfit_size) {
 		uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
 
-		pipe_w = intel_crtc->config.requested_mode.hdisplay;
-		pipe_h = intel_crtc->config.requested_mode.vdisplay;
+		pipe_w = intel_crtc->config.pipe_src_w;
+		pipe_h = intel_crtc->config.pipe_src_h;
 		pfit_w = (pfit_size >> 16) & 0xFFFF;
 		pfit_h = pfit_size & 0xFFFF;
 		if (pipe_w < pfit_w)
@@ -2640,8 +2638,7 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
 		p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
 		p->pri.bytes_per_pixel = crtc->fb->bits_per_pixel / 8;
 		p->cur.bytes_per_pixel = 4;
-		p->pri.horiz_pixels =
-			intel_crtc->config.requested_mode.hdisplay;
+		p->pri.horiz_pixels = intel_crtc->config.pipe_src_w;
 		p->cur.horiz_pixels = 64;
 		/* TODO: for now, assume primary and cursor planes are always enabled. */
 		p->pri.enabled = true;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 753cef3..71717e2 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -652,8 +652,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		.y2 = crtc_y + crtc_h,
 	};
 	const struct drm_rect clip = {
-		.x2 = intel_crtc->config.requested_mode.hdisplay,
-		.y2 = intel_crtc->config.requested_mode.vdisplay,
+		.x2 = intel_crtc->config.pipe_src_w,
+		.y2 = intel_crtc->config.pipe_src_h,
 	};
 
 	intel_fb = to_intel_framebuffer(fb);
-- 
1.8.1.5

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

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

* [PATCH 12/14] drm/i915: Document the inteded use of requested_mode
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (10 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH v2 11/14] drm/i915: Add explicit pipe src size to pipe config ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-10 17:07   ` Damien Lespiau
  2013-09-10 17:10   ` Damien Lespiau
  2013-09-04 15:25 ` [PATCH 13/14] drm/i915: Fix cursor visibility check with negative coordinates ville.syrjala
  2013-09-04 15:25 ` [PATCH 14/14] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges ville.syrjala
  13 siblings, 2 replies; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Try to clarify the purpose of requested_mode.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9d30909..2c9c96b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -211,6 +211,11 @@ struct intel_crtc_config {
 #define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (1<<0) /* unreliable sync mode.flags */
 	unsigned long quirks;
 
+	/* User requested mode, only valid as a starting point to
+	 * compute adjusted_mode, except in the case of (S)DVO where
+	 * it's also for the output timings of the (S)DVO chip.
+	 * adjusted_mode will then correspond to the S(DVO) chip's
+	 * preferred input timings. */
 	struct drm_display_mode requested_mode;
 	/* Actual pipe timings ie. what we program into the pipe timing
 	 * registers. adjusted_mode.clock is the pipe pixel clock. */
-- 
1.8.1.5

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

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

* [PATCH 13/14] drm/i915: Fix cursor visibility check with negative coordinates
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (11 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH 12/14] drm/i915: Document the inteded use of requested_mode ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-04 15:25 ` [PATCH 14/14] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges ville.syrjala
  13 siblings, 0 replies; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When the cursor x coordinate is exactly -cursor_width, the cursor is
invisible. And obviously the same holds for the y coordinate and
cursor_height.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8d64d54..20f373b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7016,7 +7016,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 		base = 0;
 
 	if (x < 0) {
-		if (x + intel_crtc->cursor_width < 0)
+		if (x + intel_crtc->cursor_width <= 0)
 			base = 0;
 
 		pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
@@ -7025,7 +7025,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	pos |= x << CURSOR_X_SHIFT;
 
 	if (y < 0) {
-		if (y + intel_crtc->cursor_height < 0)
+		if (y + intel_crtc->cursor_height <= 0)
 			base = 0;
 
 		pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
-- 
1.8.1.5

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

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

* [PATCH 14/14] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges
  2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
                   ` (12 preceding siblings ...)
  2013-09-04 15:25 ` [PATCH 13/14] drm/i915: Fix cursor visibility check with negative coordinates ville.syrjala
@ 2013-09-04 15:25 ` ville.syrjala
  2013-09-16 21:52   ` Daniel Vetter
  13 siblings, 1 reply; 34+ messages in thread
From: ville.syrjala @ 2013-09-04 15:25 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

First of all we should not be looking at fb->{width,height} as those do
not tell us what the actual pipe size is. Second of all we need to use
>= for the comparison.

So fix the comparison, and make use of the new pipe_src_{w,h} to
determine the real pipe source dimensions.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 20f373b..44e0a84 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7000,19 +7000,16 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	int pipe = intel_crtc->pipe;
 	int x = intel_crtc->cursor_x;
 	int y = intel_crtc->cursor_y;
-	u32 base, pos;
+	u32 base = 0, pos = 0;
 	bool visible;
 
-	pos = 0;
-
-	if (on && crtc->enabled && crtc->fb) {
+	if (on)
 		base = intel_crtc->cursor_addr;
-		if (x > (int) crtc->fb->width)
-			base = 0;
 
-		if (y > (int) crtc->fb->height)
-			base = 0;
-	} else
+	if (x >= intel_crtc->config.pipe_src_w)
+		base = 0;
+
+	if (y >= intel_crtc->config.pipe_src_h)
 		base = 0;
 
 	if (x < 0) {
-- 
1.8.1.5

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

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

* Re: [PATCH v2 01/14] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode
  2013-09-04 15:25 ` [PATCH v2 01/14] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode ville.syrjala
@ 2013-09-10 14:27   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 14:27 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:18PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> i9xx_set_pipeconf() attempts to get the current pixel clock from
> requested_mode. requested_mode.clock may be totally bogus, so the
> clock should come from adjusted_mode.
> 
> v2: Dropped the intel_compute_config() hunk due to killing of the
>     INTEL_FDI_FREQ check
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

If you feel brave enough you could also have reindent the comment above
the modified line to 80 chars.

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a4fe594..2c450fe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4786,7 +4786,7 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  		 * XXX: No double-wide on 915GM pipe B. Is that the only reason for the
>  		 * pipe == 0 check?
>  		 */
> -		if (intel_crtc->config.requested_mode.clock >
> +		if (intel_crtc->config.adjusted_mode.clock >
>  		    dev_priv->display.get_display_clock_speed(dev) * 9 / 10)
>  			pipeconf |= PIPECONF_DOUBLE_WIDE;
>  	}
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 02/14] drm/i915: Use adjusted_mode->clock in lpt_program_iclkip
  2013-09-04 15:25 ` [PATCH 02/14] drm/i915: Use adjusted_mode->clock in lpt_program_iclkip ville.syrjala
@ 2013-09-10 14:41   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 14:41 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:19PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> lpt_program_iclkip() wants to know the pixel clock. It should get that
> information from adjusted_mode, not crtc->mode.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2c450fe..fd67758 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2875,6 +2875,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
>  	u32 divsel, phaseinc, auxdiv, phasedir = 0;
>  	u32 temp;
>  
> @@ -2892,13 +2893,13 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
>  			SBI_ICLK);
>  
>  	/* 20MHz is a corner case which is out of range for the 7-bit divisor */
> -	if (crtc->mode.clock == 20000) {
> +	if (clock == 20000) {
>  		auxdiv = 1;
>  		divsel = 0x41;
>  		phaseinc = 0x20;
>  	} else {
>  		/* The iCLK virtual clock root frequency is in MHz,
> -		 * but the crtc->mode.clock in in KHz. To get the divisors,
> +		 * but the adjusted_mode->clock in in KHz. To get the divisors,
>  		 * it is necessary to divide one by another, so we
>  		 * convert the virtual clock precision to KHz here for higher
>  		 * precision.
> @@ -2907,7 +2908,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
>  		u32 iclk_pi_range = 64;
>  		u32 desired_divisor, msb_divisor_value, pi_value;
>  
> -		desired_divisor = (iclk_virtual_root_freq / crtc->mode.clock);
> +		desired_divisor = (iclk_virtual_root_freq / clock);
>  		msb_divisor_value = desired_divisor / iclk_pi_range;
>  		pi_value = desired_divisor % iclk_pi_range;
>  
> @@ -2923,7 +2924,7 @@ static void lpt_program_iclkip(struct drm_crtc *crtc)
>  		~SBI_SSCDIVINTPHASE_INCVAL_MASK);
>  
>  	DRM_DEBUG_KMS("iCLKIP clock: found settings for %dKHz refresh rate: auxdiv=%x, divsel=%x, phasedir=%x, phaseinc=%x\n",
> -			crtc->mode.clock,
> +			clock,
>  			auxdiv,
>  			divsel,
>  			phasedir,
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 03/14] drm/i915: Use adjusted_mode in HDMI 12bpc clock check
  2013-09-04 15:25 ` [PATCH 03/14] drm/i915: Use adjusted_mode in HDMI 12bpc clock check ville.syrjala
@ 2013-09-10 14:42   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 14:42 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:20PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The pixel clock should come from adjusted_mode not requested_mode.
> In this case the two should be the same as we don't currently
> overwrite the clock in the case of HDMI. But let's make the code
> safe against such things happening in the future.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 3d833a3..6e87b83 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -865,7 +865,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> -	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
> +	int clock_12bpc = pipe_config->adjusted_mode.clock * 3 / 2;
>  	int portclock_limit = hdmi_portclock_limit(intel_hdmi);
>  	int desired_bpp;
>  
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 04/14] drm/i915: Use adjusted_mode in intel_update_fbc()
  2013-09-04 15:25 ` [PATCH 04/14] drm/i915: Use adjusted_mode in intel_update_fbc() ville.syrjala
@ 2013-09-10 14:45   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 14:45 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:21PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Check the mode flags from the adjusted_mode, not user requested mode.
> The hdisplay/vdisplay check actually checkes the primary plane size,
> so those still need to come from the user requested mode.
> 
> Extract both modes from pipe config instead of the drm_crtc.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0c115cc..af1f4de 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -458,6 +458,8 @@ void intel_update_fbc(struct drm_device *dev)
>  	struct drm_framebuffer *fb;
>  	struct intel_framebuffer *intel_fb;
>  	struct drm_i915_gem_object *obj;
> +	const struct drm_display_mode *mode;
> +	const struct drm_display_mode *adjusted_mode;
>  	unsigned int max_hdisplay, max_vdisplay;
>  
>  	if (!I915_HAS_FBC(dev)) {
> @@ -502,6 +504,8 @@ void intel_update_fbc(struct drm_device *dev)
>  	fb = crtc->fb;
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
> +	mode = &intel_crtc->config.requested_mode;
> +	adjusted_mode = &intel_crtc->config.adjusted_mode;
>  
>  	if (i915_enable_fbc < 0 &&
>  	    INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev)) {
> @@ -514,8 +518,8 @@ void intel_update_fbc(struct drm_device *dev)
>  			DRM_DEBUG_KMS("fbc disabled per module param\n");
>  		goto out_disable;
>  	}
> -	if ((crtc->mode.flags & DRM_MODE_FLAG_INTERLACE) ||
> -	    (crtc->mode.flags & DRM_MODE_FLAG_DBLSCAN)) {
> +	if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
> +	    (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
>  		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
>  			DRM_DEBUG_KMS("mode incompatible with compression, "
>  				      "disabling\n");
> @@ -529,8 +533,8 @@ void intel_update_fbc(struct drm_device *dev)
>  		max_hdisplay = 2048;
>  		max_vdisplay = 1536;
>  	}
> -	if ((crtc->mode.hdisplay > max_hdisplay) ||
> -	    (crtc->mode.vdisplay > max_vdisplay)) {
> +	if ((mode->hdisplay > max_hdisplay) ||
> +	    (mode->vdisplay > max_vdisplay)) {
>  		if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
>  			DRM_DEBUG_KMS("mode too large for compression, disabling\n");
>  		goto out_disable;
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 05/14] drm/i915: Use adjusted_mode appropriately when computing watermarks
  2013-09-04 15:25 ` [PATCH 05/14] drm/i915: Use adjusted_mode appropriately when computing watermarks ville.syrjala
@ 2013-09-10 15:00   ` Damien Lespiau
  2013-09-10 15:04     ` Ville Syrjälä
  2013-09-10 16:08   ` Damien Lespiau
  1 sibling, 1 reply; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 15:00 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:22PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently most of the watermark code looks at crtc->mode which is the
> user requested mode. The only piece of information there that is
> relevant is hdisplay, the rest must come from adjusted_mode. Convert
> all of the code to use requested_mode and adjusted_mode from
> pipe config appropriately.

I'm not quite sure why you single out hdisplay here, would you mind
explaining why?

-- 
Damien

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 55 ++++++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index af1f4de..48d93d3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1109,7 +1109,7 @@ static void pineview_update_wm(struct drm_device *dev)
>  
>  	crtc = single_enabled_crtc(dev);
>  	if (crtc) {
> -		int clock = crtc->mode.clock;
> +		int clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
>  		int pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  		/* Display SR */
> @@ -1170,6 +1170,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
>  			    int *cursor_wm)
>  {
>  	struct drm_crtc *crtc;
> +	const struct drm_display_mode *adjusted_mode;
>  	int htotal, hdisplay, clock, pixel_size;
>  	int line_time_us, line_count;
>  	int entries, tlb_miss;
> @@ -1181,9 +1182,10 @@ static bool g4x_compute_wm0(struct drm_device *dev,
>  		return false;
>  	}
>  
> -	htotal = crtc->mode.htotal;
> -	hdisplay = crtc->mode.hdisplay;
> -	clock = crtc->mode.clock;
> +	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> +	clock = adjusted_mode->clock;
> +	htotal = adjusted_mode->htotal;
> +	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  	/* Use the small buffer method to calculate plane watermark */
> @@ -1254,6 +1256,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
>  			     int *display_wm, int *cursor_wm)
>  {
>  	struct drm_crtc *crtc;
> +	const struct drm_display_mode *adjusted_mode;
>  	int hdisplay, htotal, pixel_size, clock;
>  	unsigned long line_time_us;
>  	int line_count, line_size;
> @@ -1266,9 +1269,10 @@ static bool g4x_compute_srwm(struct drm_device *dev,
>  	}
>  
>  	crtc = intel_get_crtc_for_plane(dev, plane);
> -	hdisplay = crtc->mode.hdisplay;
> -	htotal = crtc->mode.htotal;
> -	clock = crtc->mode.clock;
> +	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> +	clock = adjusted_mode->clock;
> +	htotal = adjusted_mode->htotal;
> +	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  	line_time_us = (htotal * 1000) / clock;
> @@ -1307,7 +1311,7 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
>  	if (!intel_crtc_active(crtc))
>  		return false;
>  
> -	clock = crtc->mode.clock;	/* VESA DOT Clock */
> +	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;	/* BPP */
>  
>  	entries = (clock / 1000) * pixel_size;
> @@ -1492,9 +1496,11 @@ static void i965_update_wm(struct drm_device *dev)
>  	if (crtc) {
>  		/* self-refresh has much higher latency */
>  		static const int sr_latency_ns = 12000;
> -		int clock = crtc->mode.clock;
> -		int htotal = crtc->mode.htotal;
> -		int hdisplay = crtc->mode.hdisplay;
> +		const struct drm_display_mode *adjusted_mode =
> +			&to_intel_crtc(crtc)->config.adjusted_mode;
> +		int clock = adjusted_mode->clock;
> +		int htotal = adjusted_mode->htotal;
> +		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  		int pixel_size = crtc->fb->bits_per_pixel / 8;
>  		unsigned long line_time_us;
>  		int entries;
> @@ -1570,7 +1576,7 @@ static void i9xx_update_wm(struct drm_device *dev)
>  		if (IS_GEN2(dev))
>  			cpp = 4;
>  
> -		planea_wm = intel_calculate_wm(crtc->mode.clock,
> +		planea_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
>  					       wm_info, fifo_size, cpp,
>  					       latency_ns);
>  		enabled = crtc;
> @@ -1584,7 +1590,7 @@ static void i9xx_update_wm(struct drm_device *dev)
>  		if (IS_GEN2(dev))
>  			cpp = 4;
>  
> -		planeb_wm = intel_calculate_wm(crtc->mode.clock,
> +		planeb_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
>  					       wm_info, fifo_size, cpp,
>  					       latency_ns);
>  		if (enabled == NULL)
> @@ -1611,9 +1617,11 @@ static void i9xx_update_wm(struct drm_device *dev)
>  	if (HAS_FW_BLC(dev) && enabled) {
>  		/* self-refresh has much higher latency */
>  		static const int sr_latency_ns = 6000;
> -		int clock = enabled->mode.clock;
> -		int htotal = enabled->mode.htotal;
> -		int hdisplay = enabled->mode.hdisplay;
> +		const struct drm_display_mode *adjusted_mode =
> +			&to_intel_crtc(enabled)->config.adjusted_mode;
> +		int clock = adjusted_mode->clock;
> +		int htotal = adjusted_mode->htotal;
> +		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  		int pixel_size = enabled->fb->bits_per_pixel / 8;
>  		unsigned long line_time_us;
>  		int entries;
> @@ -1673,7 +1681,8 @@ static void i830_update_wm(struct drm_device *dev)
>  	if (crtc == NULL)
>  		return;
>  
> -	planea_wm = intel_calculate_wm(crtc->mode.clock, &i830_wm_info,
> +	planea_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
> +				       &i830_wm_info,
>  				       dev_priv->display.get_fifo_size(dev, 0),
>  				       4, latency_ns);
>  	fwater_lo = I915_READ(FW_BLC) & ~0xfff;
> @@ -1745,6 +1754,7 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
>  				  int *fbc_wm, int *display_wm, int *cursor_wm)
>  {
>  	struct drm_crtc *crtc;
> +	const struct drm_display_mode *adjusted_mode;
>  	unsigned long line_time_us;
>  	int hdisplay, htotal, pixel_size, clock;
>  	int line_count, line_size;
> @@ -1757,9 +1767,10 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
>  	}
>  
>  	crtc = intel_get_crtc_for_plane(dev, plane);
> -	hdisplay = crtc->mode.hdisplay;
> -	htotal = crtc->mode.htotal;
> -	clock = crtc->mode.clock;
> +	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> +	clock = adjusted_mode->clock;
> +	htotal = adjusted_mode->htotal;
> +	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  	line_time_us = (htotal * 1000) / clock;
> @@ -2902,7 +2913,7 @@ sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
>  		return false;
>  	}
>  
> -	clock = crtc->mode.clock;
> +	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
>  
>  	/* Use the small buffer method to calculate the sprite watermark */
>  	entries = ((clock * pixel_size / 1000) * display_latency_ns) / 1000;
> @@ -2937,7 +2948,7 @@ sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
>  	}
>  
>  	crtc = intel_get_crtc_for_plane(dev, plane);
> -	clock = crtc->mode.clock;
> +	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
>  	if (!clock) {
>  		*sprite_wm = 0;
>  		return false;
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 06/14] drm/i915: Check the clock from adjusted mode in intel_crtc_active()
  2013-09-04 15:25 ` [PATCH 06/14] drm/i915: Check the clock from adjusted mode in intel_crtc_active() ville.syrjala
@ 2013-09-10 15:00   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 15:00 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:23PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The clock in crtc->mode doesn't necessarily mean anything. Let's look
> at the clock in adjusted_mode instead.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 48d93d3..397628b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -45,10 +45,13 @@
>  
>  static bool intel_crtc_active(struct drm_crtc *crtc)
>  {
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
>  	/* Be paranoid as we can arrive here with only partial
>  	 * state retrieved from the hardware during setup.
>  	 */
> -	return to_intel_crtc(crtc)->active && crtc->fb && crtc->mode.clock;
> +	return intel_crtc->active && crtc->fb &&
> +		intel_crtc->config.adjusted_mode.clock;
>  }
>  
>  static void i8xx_disable_fbc(struct drm_device *dev)
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 05/14] drm/i915: Use adjusted_mode appropriately when computing watermarks
  2013-09-10 15:00   ` Damien Lespiau
@ 2013-09-10 15:04     ` Ville Syrjälä
  0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2013-09-10 15:04 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Tue, Sep 10, 2013 at 04:00:03PM +0100, Damien Lespiau wrote:
> On Wed, Sep 04, 2013 at 06:25:22PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently most of the watermark code looks at crtc->mode which is the
> > user requested mode. The only piece of information there that is
> > relevant is hdisplay, the rest must come from adjusted_mode. Convert
> > all of the code to use requested_mode and adjusted_mode from
> > pipe config appropriately.
> 
> I'm not quite sure why you single out hdisplay here, would you mind
> explaining why?

The watermark code is usually interested in the primary plane width.
requested_mode.hdisplay is the closest thing we have at the momemnt.

> 
> -- 
> Damien
> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 55 ++++++++++++++++++++++++-----------------
> >  1 file changed, 33 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index af1f4de..48d93d3 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1109,7 +1109,7 @@ static void pineview_update_wm(struct drm_device *dev)
> >  
> >  	crtc = single_enabled_crtc(dev);
> >  	if (crtc) {
> > -		int clock = crtc->mode.clock;
> > +		int clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
> >  		int pixel_size = crtc->fb->bits_per_pixel / 8;
> >  
> >  		/* Display SR */
> > @@ -1170,6 +1170,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> >  			    int *cursor_wm)
> >  {
> >  	struct drm_crtc *crtc;
> > +	const struct drm_display_mode *adjusted_mode;
> >  	int htotal, hdisplay, clock, pixel_size;
> >  	int line_time_us, line_count;
> >  	int entries, tlb_miss;
> > @@ -1181,9 +1182,10 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> >  		return false;
> >  	}
> >  
> > -	htotal = crtc->mode.htotal;
> > -	hdisplay = crtc->mode.hdisplay;
> > -	clock = crtc->mode.clock;
> > +	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> > +	clock = adjusted_mode->clock;
> > +	htotal = adjusted_mode->htotal;
> > +	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> >  	pixel_size = crtc->fb->bits_per_pixel / 8;
> >  
> >  	/* Use the small buffer method to calculate plane watermark */
> > @@ -1254,6 +1256,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> >  			     int *display_wm, int *cursor_wm)
> >  {
> >  	struct drm_crtc *crtc;
> > +	const struct drm_display_mode *adjusted_mode;
> >  	int hdisplay, htotal, pixel_size, clock;
> >  	unsigned long line_time_us;
> >  	int line_count, line_size;
> > @@ -1266,9 +1269,10 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> >  	}
> >  
> >  	crtc = intel_get_crtc_for_plane(dev, plane);
> > -	hdisplay = crtc->mode.hdisplay;
> > -	htotal = crtc->mode.htotal;
> > -	clock = crtc->mode.clock;
> > +	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> > +	clock = adjusted_mode->clock;
> > +	htotal = adjusted_mode->htotal;
> > +	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> >  	pixel_size = crtc->fb->bits_per_pixel / 8;
> >  
> >  	line_time_us = (htotal * 1000) / clock;
> > @@ -1307,7 +1311,7 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
> >  	if (!intel_crtc_active(crtc))
> >  		return false;
> >  
> > -	clock = crtc->mode.clock;	/* VESA DOT Clock */
> > +	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
> >  	pixel_size = crtc->fb->bits_per_pixel / 8;	/* BPP */
> >  
> >  	entries = (clock / 1000) * pixel_size;
> > @@ -1492,9 +1496,11 @@ static void i965_update_wm(struct drm_device *dev)
> >  	if (crtc) {
> >  		/* self-refresh has much higher latency */
> >  		static const int sr_latency_ns = 12000;
> > -		int clock = crtc->mode.clock;
> > -		int htotal = crtc->mode.htotal;
> > -		int hdisplay = crtc->mode.hdisplay;
> > +		const struct drm_display_mode *adjusted_mode =
> > +			&to_intel_crtc(crtc)->config.adjusted_mode;
> > +		int clock = adjusted_mode->clock;
> > +		int htotal = adjusted_mode->htotal;
> > +		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> >  		int pixel_size = crtc->fb->bits_per_pixel / 8;
> >  		unsigned long line_time_us;
> >  		int entries;
> > @@ -1570,7 +1576,7 @@ static void i9xx_update_wm(struct drm_device *dev)
> >  		if (IS_GEN2(dev))
> >  			cpp = 4;
> >  
> > -		planea_wm = intel_calculate_wm(crtc->mode.clock,
> > +		planea_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
> >  					       wm_info, fifo_size, cpp,
> >  					       latency_ns);
> >  		enabled = crtc;
> > @@ -1584,7 +1590,7 @@ static void i9xx_update_wm(struct drm_device *dev)
> >  		if (IS_GEN2(dev))
> >  			cpp = 4;
> >  
> > -		planeb_wm = intel_calculate_wm(crtc->mode.clock,
> > +		planeb_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
> >  					       wm_info, fifo_size, cpp,
> >  					       latency_ns);
> >  		if (enabled == NULL)
> > @@ -1611,9 +1617,11 @@ static void i9xx_update_wm(struct drm_device *dev)
> >  	if (HAS_FW_BLC(dev) && enabled) {
> >  		/* self-refresh has much higher latency */
> >  		static const int sr_latency_ns = 6000;
> > -		int clock = enabled->mode.clock;
> > -		int htotal = enabled->mode.htotal;
> > -		int hdisplay = enabled->mode.hdisplay;
> > +		const struct drm_display_mode *adjusted_mode =
> > +			&to_intel_crtc(enabled)->config.adjusted_mode;
> > +		int clock = adjusted_mode->clock;
> > +		int htotal = adjusted_mode->htotal;
> > +		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> >  		int pixel_size = enabled->fb->bits_per_pixel / 8;
> >  		unsigned long line_time_us;
> >  		int entries;
> > @@ -1673,7 +1681,8 @@ static void i830_update_wm(struct drm_device *dev)
> >  	if (crtc == NULL)
> >  		return;
> >  
> > -	planea_wm = intel_calculate_wm(crtc->mode.clock, &i830_wm_info,
> > +	planea_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
> > +				       &i830_wm_info,
> >  				       dev_priv->display.get_fifo_size(dev, 0),
> >  				       4, latency_ns);
> >  	fwater_lo = I915_READ(FW_BLC) & ~0xfff;
> > @@ -1745,6 +1754,7 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
> >  				  int *fbc_wm, int *display_wm, int *cursor_wm)
> >  {
> >  	struct drm_crtc *crtc;
> > +	const struct drm_display_mode *adjusted_mode;
> >  	unsigned long line_time_us;
> >  	int hdisplay, htotal, pixel_size, clock;
> >  	int line_count, line_size;
> > @@ -1757,9 +1767,10 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
> >  	}
> >  
> >  	crtc = intel_get_crtc_for_plane(dev, plane);
> > -	hdisplay = crtc->mode.hdisplay;
> > -	htotal = crtc->mode.htotal;
> > -	clock = crtc->mode.clock;
> > +	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> > +	clock = adjusted_mode->clock;
> > +	htotal = adjusted_mode->htotal;
> > +	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> >  	pixel_size = crtc->fb->bits_per_pixel / 8;
> >  
> >  	line_time_us = (htotal * 1000) / clock;
> > @@ -2902,7 +2913,7 @@ sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
> >  		return false;
> >  	}
> >  
> > -	clock = crtc->mode.clock;
> > +	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
> >  
> >  	/* Use the small buffer method to calculate the sprite watermark */
> >  	entries = ((clock * pixel_size / 1000) * display_latency_ns) / 1000;
> > @@ -2937,7 +2948,7 @@ sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
> >  	}
> >  
> >  	crtc = intel_get_crtc_for_plane(dev, plane);
> > -	clock = crtc->mode.clock;
> > +	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
> >  	if (!clock) {
> >  		*sprite_wm = 0;
> >  		return false;
> > -- 
> > 1.8.1.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

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

* Re: [PATCH 05/14] drm/i915: Use adjusted_mode appropriately when computing watermarks
  2013-09-04 15:25 ` [PATCH 05/14] drm/i915: Use adjusted_mode appropriately when computing watermarks ville.syrjala
  2013-09-10 15:00   ` Damien Lespiau
@ 2013-09-10 16:08   ` Damien Lespiau
  1 sibling, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 16:08 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:22PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently most of the watermark code looks at crtc->mode which is the
> user requested mode. The only piece of information there that is
> relevant is hdisplay, the rest must come from adjusted_mode. Convert
> all of the code to use requested_mode and adjusted_mode from
> pipe config appropriately.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 55 ++++++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index af1f4de..48d93d3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1109,7 +1109,7 @@ static void pineview_update_wm(struct drm_device *dev)
>  
>  	crtc = single_enabled_crtc(dev);
>  	if (crtc) {
> -		int clock = crtc->mode.clock;
> +		int clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
>  		int pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  		/* Display SR */
> @@ -1170,6 +1170,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
>  			    int *cursor_wm)
>  {
>  	struct drm_crtc *crtc;
> +	const struct drm_display_mode *adjusted_mode;
>  	int htotal, hdisplay, clock, pixel_size;
>  	int line_time_us, line_count;
>  	int entries, tlb_miss;
> @@ -1181,9 +1182,10 @@ static bool g4x_compute_wm0(struct drm_device *dev,
>  		return false;
>  	}
>  
> -	htotal = crtc->mode.htotal;
> -	hdisplay = crtc->mode.hdisplay;
> -	clock = crtc->mode.clock;
> +	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> +	clock = adjusted_mode->clock;
> +	htotal = adjusted_mode->htotal;
> +	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  	/* Use the small buffer method to calculate plane watermark */
> @@ -1254,6 +1256,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
>  			     int *display_wm, int *cursor_wm)
>  {
>  	struct drm_crtc *crtc;
> +	const struct drm_display_mode *adjusted_mode;
>  	int hdisplay, htotal, pixel_size, clock;
>  	unsigned long line_time_us;
>  	int line_count, line_size;
> @@ -1266,9 +1269,10 @@ static bool g4x_compute_srwm(struct drm_device *dev,
>  	}
>  
>  	crtc = intel_get_crtc_for_plane(dev, plane);
> -	hdisplay = crtc->mode.hdisplay;
> -	htotal = crtc->mode.htotal;
> -	clock = crtc->mode.clock;
> +	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> +	clock = adjusted_mode->clock;
> +	htotal = adjusted_mode->htotal;
> +	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  	line_time_us = (htotal * 1000) / clock;
> @@ -1307,7 +1311,7 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
>  	if (!intel_crtc_active(crtc))
>  		return false;
>  
> -	clock = crtc->mode.clock;	/* VESA DOT Clock */
> +	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;	/* BPP */
>  
>  	entries = (clock / 1000) * pixel_size;
> @@ -1492,9 +1496,11 @@ static void i965_update_wm(struct drm_device *dev)
>  	if (crtc) {
>  		/* self-refresh has much higher latency */
>  		static const int sr_latency_ns = 12000;
> -		int clock = crtc->mode.clock;
> -		int htotal = crtc->mode.htotal;
> -		int hdisplay = crtc->mode.hdisplay;
> +		const struct drm_display_mode *adjusted_mode =
> +			&to_intel_crtc(crtc)->config.adjusted_mode;
> +		int clock = adjusted_mode->clock;
> +		int htotal = adjusted_mode->htotal;
> +		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  		int pixel_size = crtc->fb->bits_per_pixel / 8;
>  		unsigned long line_time_us;
>  		int entries;
> @@ -1570,7 +1576,7 @@ static void i9xx_update_wm(struct drm_device *dev)
>  		if (IS_GEN2(dev))
>  			cpp = 4;
>  
> -		planea_wm = intel_calculate_wm(crtc->mode.clock,
> +		planea_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
>  					       wm_info, fifo_size, cpp,
>  					       latency_ns);
>  		enabled = crtc;
> @@ -1584,7 +1590,7 @@ static void i9xx_update_wm(struct drm_device *dev)
>  		if (IS_GEN2(dev))
>  			cpp = 4;
>  
> -		planeb_wm = intel_calculate_wm(crtc->mode.clock,
> +		planeb_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
>  					       wm_info, fifo_size, cpp,
>  					       latency_ns);
>  		if (enabled == NULL)
> @@ -1611,9 +1617,11 @@ static void i9xx_update_wm(struct drm_device *dev)
>  	if (HAS_FW_BLC(dev) && enabled) {
>  		/* self-refresh has much higher latency */
>  		static const int sr_latency_ns = 6000;
> -		int clock = enabled->mode.clock;
> -		int htotal = enabled->mode.htotal;
> -		int hdisplay = enabled->mode.hdisplay;
> +		const struct drm_display_mode *adjusted_mode =
> +			&to_intel_crtc(enabled)->config.adjusted_mode;
> +		int clock = adjusted_mode->clock;
> +		int htotal = adjusted_mode->htotal;
> +		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  		int pixel_size = enabled->fb->bits_per_pixel / 8;
>  		unsigned long line_time_us;
>  		int entries;
> @@ -1673,7 +1681,8 @@ static void i830_update_wm(struct drm_device *dev)
>  	if (crtc == NULL)
>  		return;
>  
> -	planea_wm = intel_calculate_wm(crtc->mode.clock, &i830_wm_info,
> +	planea_wm = intel_calculate_wm(to_intel_crtc(crtc)->config.adjusted_mode.clock,
> +				       &i830_wm_info,
>  				       dev_priv->display.get_fifo_size(dev, 0),
>  				       4, latency_ns);
>  	fwater_lo = I915_READ(FW_BLC) & ~0xfff;
> @@ -1745,6 +1754,7 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
>  				  int *fbc_wm, int *display_wm, int *cursor_wm)
>  {
>  	struct drm_crtc *crtc;
> +	const struct drm_display_mode *adjusted_mode;
>  	unsigned long line_time_us;
>  	int hdisplay, htotal, pixel_size, clock;
>  	int line_count, line_size;
> @@ -1757,9 +1767,10 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
>  	}
>  
>  	crtc = intel_get_crtc_for_plane(dev, plane);
> -	hdisplay = crtc->mode.hdisplay;
> -	htotal = crtc->mode.htotal;
> -	clock = crtc->mode.clock;
> +	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> +	clock = adjusted_mode->clock;
> +	htotal = adjusted_mode->htotal;
> +	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  	line_time_us = (htotal * 1000) / clock;
> @@ -2902,7 +2913,7 @@ sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
>  		return false;
>  	}
>  
> -	clock = crtc->mode.clock;
> +	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
>  
>  	/* Use the small buffer method to calculate the sprite watermark */
>  	entries = ((clock * pixel_size / 1000) * display_latency_ns) / 1000;
> @@ -2937,7 +2948,7 @@ sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
>  	}
>  
>  	crtc = intel_get_crtc_for_plane(dev, plane);
> -	clock = crtc->mode.clock;
> +	clock = to_intel_crtc(crtc)->config.adjusted_mode.clock;
>  	if (!clock) {
>  		*sprite_wm = 0;
>  		return false;
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 07/14] drm/i915: Use adjusted_mode when checking conditions for PSR
  2013-09-04 15:25 ` [PATCH 07/14] drm/i915: Use adjusted_mode when checking conditions for PSR ville.syrjala
@ 2013-09-10 16:42   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 16:42 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:24PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_edp_psr_match_conditions() currently looks at crtc->mode
> when it really needs to look at adjusted_mode. Fix it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c192dbb..3f0dd65 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1570,7 +1570,8 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>  	}
>  
>  	intel_crtc = to_intel_crtc(crtc);
> -	if (!intel_crtc->active || !crtc->fb || !crtc->mode.clock) {
> +	if (!intel_crtc->active || !crtc->fb ||
> +	    !intel_crtc->config.adjusted_mode.clock) {
>  		DRM_DEBUG_KMS("crtc not active for PSR\n");
>  		dev_priv->no_psr_reason = PSR_CRTC_NOT_ACTIVE;
>  		return false;
> @@ -1597,7 +1598,7 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>  		return false;
>  	}
>  
> -	if (crtc->mode.flags & DRM_MODE_FLAG_INTERLACE) {
> +	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
>  		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
>  		dev_priv->no_psr_reason = PSR_INTERLACED_ENABLED;
>  		return false;
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 08/14] drm/i915: Make intel_crtc_active() available outside intel_pm.c
  2013-09-04 15:25 ` [PATCH v2 08/14] drm/i915: Make intel_crtc_active() available outside intel_pm.c ville.syrjala
@ 2013-09-10 16:43   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 16:43 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:25PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move intel_crtc_active() to intel_display.c and make it available
> elsewhere as well.
> 
> intel_edp_psr_match_conditions() already has one open coded copy,
> so replace that one with a call to intel_crtc_active().
> 
> v2: Copy paste a big comment from danvet's mail explaining
>     when we can ditch the extra checks
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c      |  3 +--
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_pm.c      | 11 -----------
>  4 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fd67758..71c7763 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -733,6 +733,23 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
>  	return true;
>  }
>  
> +bool intel_crtc_active(struct drm_crtc *crtc)
> +{
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	/* Be paranoid as we can arrive here with only partial
> +	 * state retrieved from the hardware during setup.
> +	 *
> +	 * We can ditch the adjusted_mode.clock check as soon
> +	 * as Haswell has gained clock readout/fastboot support.
> +	 *
> +	 * We can ditch the crtc->fb check as soon as we can
> +	 * properly reconstruct framebuffers.
> +	 */
> +	return intel_crtc->active && crtc->fb &&
> +		intel_crtc->config.adjusted_mode.clock;
> +}
> +
>  enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
>  					     enum pipe pipe)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3f0dd65..87b03b2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1570,8 +1570,7 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>  	}
>  
>  	intel_crtc = to_intel_crtc(crtc);
> -	if (!intel_crtc->active || !crtc->fb ||
> -	    !intel_crtc->config.adjusted_mode.clock) {
> +	if (!intel_crtc_active(crtc)) {
>  		DRM_DEBUG_KMS("crtc not active for PSR\n");
>  		dev_priv->no_psr_reason = PSR_CRTC_NOT_ACTIVE;
>  		return false;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index dbf04be..59f0ec7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -801,5 +801,6 @@ extern void hsw_pc8_disable_interrupts(struct drm_device *dev);
>  extern void hsw_pc8_restore_interrupts(struct drm_device *dev);
>  extern void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>  extern void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
> +extern bool intel_crtc_active(struct drm_crtc *crtc);
>  
>  #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 397628b..3ba412c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -43,17 +43,6 @@
>   * i915.i915_enable_fbc parameter
>   */
>  
> -static bool intel_crtc_active(struct drm_crtc *crtc)
> -{
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -
> -	/* Be paranoid as we can arrive here with only partial
> -	 * state retrieved from the hardware during setup.
> -	 */
> -	return intel_crtc->active && crtc->fb &&
> -		intel_crtc->config.adjusted_mode.clock;
> -}
> -
>  static void i8xx_disable_fbc(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 09/14] drm/i915: Use pipe config in sprite code
  2013-09-04 15:25 ` [PATCH 09/14] drm/i915: Use pipe config in sprite code ville.syrjala
@ 2013-09-10 16:44   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 16:44 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:26PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather than dig up the pipe source size from crtc->mode, use
> intel_crtc->config.requested_mode.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index ad6ec4b..753cef3 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -652,8 +652,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		.y2 = crtc_y + crtc_h,
>  	};
>  	const struct drm_rect clip = {
> -		.x2 = crtc->mode.hdisplay,
> -		.y2 = crtc->mode.vdisplay,
> +		.x2 = intel_crtc->config.requested_mode.hdisplay,
> +		.y2 = intel_crtc->config.requested_mode.vdisplay,
>  	};
>  
>  	intel_fb = to_intel_framebuffer(fb);
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 10/14] drm/i915: Use adjusted_mode in DSI PLL calculations
  2013-09-04 15:25 ` [PATCH 10/14] drm/i915: Use adjusted_mode in DSI PLL calculations ville.syrjala
@ 2013-09-10 16:45   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 16:45 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:27PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> adjusted_mode contains our real timings, not requested_mode. Use the
> correct thing in DSI PLL code.
> 
> Also constify adjusted_mode since we don't change it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_dsi_pll.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 582f626..44279b2 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -50,7 +50,7 @@ static const u32 lfsr_converts[] = {
>  	71, 35							/* 91 - 92 */
>  };
>  
> -static u32 dsi_rr_formula(struct drm_display_mode *mode,
> +static u32 dsi_rr_formula(const struct drm_display_mode *mode,
>  			  int pixel_format, int video_mode_format,
>  			  int lane_count, bool eotp)
>  {
> @@ -245,7 +245,7 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> -	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
> +	const struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	int ret;
>  	struct dsi_mnp dsi_mnp;
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 11/14] drm/i915: Add explicit pipe src size to pipe config
  2013-09-04 15:25 ` [PATCH v2 11/14] drm/i915: Add explicit pipe src size to pipe config ville.syrjala
@ 2013-09-10 17:02   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 17:02 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:28PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather that mess about with hdisplay/vdisplay from requested_mode, add
> explicit pipe src size information to pipe config.
> 
> Now requested_mode is only really relevant for dvo/sdvo output timings.
> For everything else either adjusted_mode or pipe src size should be
> used.
> 
> In many places where we end up using pipe source size, we should
> actually use the primary plane size, but we don't currently store
> that information explicitly. As long as we treat primaries as full
> screen only, we can get away with this. Eventually when we move
> primaries over to drm_plane, we need to fix it all up.
> 
> v2: Add a comment to explain what pipe_src_{w,h} are
>     Add a note about primary planes to commit message
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h     |  6 ++++
>  drivers/gpu/drm/i915/intel_panel.c   | 56 +++++++++++++++++-------------------
>  drivers/gpu/drm/i915/intel_pm.c      | 33 ++++++++++-----------
>  drivers/gpu/drm/i915/intel_sprite.c  |  4 +--
>  5 files changed, 67 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 71c7763..8d64d54 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4673,7 +4673,6 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  	enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
>  	struct drm_display_mode *adjusted_mode =
>  		&intel_crtc->config.adjusted_mode;
> -	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
>  	uint32_t vsyncshift, crtc_vtotal, crtc_vblank_end;
>  
>  	/* We need to be careful not to changed the adjusted mode, for otherwise
> @@ -4726,7 +4725,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  	 * always be the user's requested size.
>  	 */
>  	I915_WRITE(PIPESRC(pipe),
> -		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
> +		   ((intel_crtc->config.pipe_src_w - 1) << 16) |
> +		   (intel_crtc->config.pipe_src_h - 1));
>  }
>  
>  static void intel_get_pipe_timings(struct intel_crtc *crtc,
> @@ -4764,8 +4764,11 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  	}
>  
>  	tmp = I915_READ(PIPESRC(crtc->pipe));
> -	pipe_config->requested_mode.vdisplay = (tmp & 0xffff) + 1;
> -	pipe_config->requested_mode.hdisplay = ((tmp >> 16) & 0xffff) + 1;
> +	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> +	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> +
> +	pipe_config->requested_mode.vdisplay = pipe_config->pipe_src_h;
> +	pipe_config->requested_mode.hdisplay = pipe_config->pipe_src_w;
>  }
>  
>  static void intel_crtc_mode_from_pipe_config(struct intel_crtc *intel_crtc,
> @@ -4861,7 +4864,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
>  	int refclk, num_connectors = 0;
> @@ -4960,8 +4962,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	 * which should always be the user's requested size.
>  	 */
>  	I915_WRITE(DSPSIZE(plane),
> -		   ((mode->vdisplay - 1) << 16) |
> -		   (mode->hdisplay - 1));
> +		   ((intel_crtc->config.pipe_src_h - 1) << 16) |
> +		   (intel_crtc->config.pipe_src_w - 1));
>  	I915_WRITE(DSPPOS(plane), 0);
>  
>  	i9xx_set_pipeconf(intel_crtc);
> @@ -8281,6 +8283,8 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  	DRM_DEBUG_KMS("adjusted mode:\n");
>  	drm_mode_debug_printmodeline(&pipe_config->adjusted_mode);
>  	DRM_DEBUG_KMS("port clock: %d\n", pipe_config->port_clock);
> +	DRM_DEBUG_KMS("pipe src size: %dx%d\n",
> +		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
>  	DRM_DEBUG_KMS("gmch pfit: control: 0x%08x, ratios: 0x%08x, lvds border: 0x%08x\n",
>  		      pipe_config->gmch_pfit.control,
>  		      pipe_config->gmch_pfit.pgm_ratios,
> @@ -8332,6 +8336,10 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  
>  	drm_mode_copy(&pipe_config->adjusted_mode, mode);
>  	drm_mode_copy(&pipe_config->requested_mode, mode);
> +
> +	pipe_config->pipe_src_w = mode->hdisplay;
> +	pipe_config->pipe_src_h = mode->vdisplay;
> +
>  	pipe_config->cpu_transcoder =
>  		(enum transcoder) to_intel_crtc(crtc)->pipe;
>  	pipe_config->shared_dpll = DPLL_ID_PRIVATE;
> @@ -8681,8 +8689,8 @@ intel_pipe_config_compare(struct drm_device *dev,
>  				      DRM_MODE_FLAG_NVSYNC);
>  	}
>  
> -	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
> -	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
> +	PIPE_CONF_CHECK_I(pipe_src_w);
> +	PIPE_CONF_CHECK_I(pipe_src_h);
>  
>  	PIPE_CONF_CHECK_I(gmch_pfit.control);
>  	/* pfit ratios are autocomputed by the hw on gen4+ */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 59f0ec7..9d30909 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -215,6 +215,12 @@ struct intel_crtc_config {
>  	/* Actual pipe timings ie. what we program into the pipe timing
>  	 * registers. adjusted_mode.clock is the pipe pixel clock. */
>  	struct drm_display_mode adjusted_mode;
> +
> +	/* Pipe source size (ie. panel fitter input size)
> +	 * All planes will be positioned inside this space,
> +	 * and get clipped at the edges. */
> +	int pipe_src_w, pipe_src_h;
> +
>  	/* Whether to set up the PCH/FDI. Note that we never allow sharing
>  	 * between pch encoders and cpu encoders. */
>  	bool has_pch_encoder;
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 42114ec..c9dba46 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -50,23 +50,22 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
>  			struct intel_crtc_config *pipe_config,
>  			int fitting_mode)
>  {
> -	struct drm_display_mode *mode, *adjusted_mode;
> +	struct drm_display_mode *adjusted_mode;
>  	int x, y, width, height;
>  
> -	mode = &pipe_config->requested_mode;
>  	adjusted_mode = &pipe_config->adjusted_mode;
>  
>  	x = y = width = height = 0;
>  
>  	/* Native modes don't need fitting */
> -	if (adjusted_mode->hdisplay == mode->hdisplay &&
> -	    adjusted_mode->vdisplay == mode->vdisplay)
> +	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> +	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
>  		goto done;
>  
>  	switch (fitting_mode) {
>  	case DRM_MODE_SCALE_CENTER:
> -		width = mode->hdisplay;
> -		height = mode->vdisplay;
> +		width = pipe_config->pipe_src_w;
> +		height = pipe_config->pipe_src_h;
>  		x = (adjusted_mode->hdisplay - width + 1)/2;
>  		y = (adjusted_mode->vdisplay - height + 1)/2;
>  		break;
> @@ -74,17 +73,17 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
>  	case DRM_MODE_SCALE_ASPECT:
>  		/* Scale but preserve the aspect ratio */
>  		{
> -			u32 scaled_width = adjusted_mode->hdisplay * mode->vdisplay;
> -			u32 scaled_height = mode->hdisplay * adjusted_mode->vdisplay;
> +			u32 scaled_width = adjusted_mode->hdisplay * pipe_config->pipe_src_h;
> +			u32 scaled_height = pipe_config->pipe_src_w * adjusted_mode->vdisplay;
>  			if (scaled_width > scaled_height) { /* pillar */
> -				width = scaled_height / mode->vdisplay;
> +				width = scaled_height / pipe_config->pipe_src_h;
>  				if (width & 1)
>  					width++;
>  				x = (adjusted_mode->hdisplay - width + 1) / 2;
>  				y = 0;
>  				height = adjusted_mode->vdisplay;
>  			} else if (scaled_width < scaled_height) { /* letter */
> -				height = scaled_width / mode->hdisplay;
> +				height = scaled_width / pipe_config->pipe_src_w;
>  				if (height & 1)
>  				    height++;
>  				y = (adjusted_mode->vdisplay - height + 1) / 2;
> @@ -176,14 +175,13 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	u32 pfit_control = 0, pfit_pgm_ratios = 0, border = 0;
> -	struct drm_display_mode *mode, *adjusted_mode;
> +	struct drm_display_mode *adjusted_mode;
>  
> -	mode = &pipe_config->requested_mode;
>  	adjusted_mode = &pipe_config->adjusted_mode;
>  
>  	/* Native modes don't need fitting */
> -	if (adjusted_mode->hdisplay == mode->hdisplay &&
> -	    adjusted_mode->vdisplay == mode->vdisplay)
> +	if (adjusted_mode->hdisplay == pipe_config->pipe_src_w &&
> +	    adjusted_mode->vdisplay == pipe_config->pipe_src_h)
>  		goto out;
>  
>  	switch (fitting_mode) {
> @@ -192,16 +190,16 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  		 * For centered modes, we have to calculate border widths &
>  		 * heights and modify the values programmed into the CRTC.
>  		 */
> -		centre_horizontally(adjusted_mode, mode->hdisplay);
> -		centre_vertically(adjusted_mode, mode->vdisplay);
> +		centre_horizontally(adjusted_mode, pipe_config->pipe_src_w);
> +		centre_vertically(adjusted_mode, pipe_config->pipe_src_h);
>  		border = LVDS_BORDER_ENABLE;
>  		break;
>  	case DRM_MODE_SCALE_ASPECT:
>  		/* Scale but preserve the aspect ratio */
>  		if (INTEL_INFO(dev)->gen >= 4) {
>  			u32 scaled_width = adjusted_mode->hdisplay *
> -				mode->vdisplay;
> -			u32 scaled_height = mode->hdisplay *
> +				pipe_config->pipe_src_h;
> +			u32 scaled_height = pipe_config->pipe_src_w *
>  				adjusted_mode->vdisplay;
>  
>  			/* 965+ is easy, it does everything in hw */
> @@ -211,12 +209,12 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  			else if (scaled_width < scaled_height)
>  				pfit_control |= PFIT_ENABLE |
>  					PFIT_SCALING_LETTER;
> -			else if (adjusted_mode->hdisplay != mode->hdisplay)
> +			else if (adjusted_mode->hdisplay != pipe_config->pipe_src_w)
>  				pfit_control |= PFIT_ENABLE | PFIT_SCALING_AUTO;
>  		} else {
>  			u32 scaled_width = adjusted_mode->hdisplay *
> -				mode->vdisplay;
> -			u32 scaled_height = mode->hdisplay *
> +				pipe_config->pipe_src_h;
> +			u32 scaled_height = pipe_config->pipe_src_w *
>  				adjusted_mode->vdisplay;
>  			/*
>  			 * For earlier chips we have to calculate the scaling
> @@ -226,11 +224,11 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  			if (scaled_width > scaled_height) { /* pillar */
>  				centre_horizontally(adjusted_mode,
>  						    scaled_height /
> -						    mode->vdisplay);
> +						    pipe_config->pipe_src_h);
>  
>  				border = LVDS_BORDER_ENABLE;
> -				if (mode->vdisplay != adjusted_mode->vdisplay) {
> -					u32 bits = panel_fitter_scaling(mode->vdisplay, adjusted_mode->vdisplay);
> +				if (pipe_config->pipe_src_h != adjusted_mode->vdisplay) {
> +					u32 bits = panel_fitter_scaling(pipe_config->pipe_src_h, adjusted_mode->vdisplay);
>  					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
>  							    bits << PFIT_VERT_SCALE_SHIFT);
>  					pfit_control |= (PFIT_ENABLE |
> @@ -240,11 +238,11 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  			} else if (scaled_width < scaled_height) { /* letter */
>  				centre_vertically(adjusted_mode,
>  						  scaled_width /
> -						  mode->hdisplay);
> +						  pipe_config->pipe_src_w);
>  
>  				border = LVDS_BORDER_ENABLE;
> -				if (mode->hdisplay != adjusted_mode->hdisplay) {
> -					u32 bits = panel_fitter_scaling(mode->hdisplay, adjusted_mode->hdisplay);
> +				if (pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
> +					u32 bits = panel_fitter_scaling(pipe_config->pipe_src_w, adjusted_mode->hdisplay);
>  					pfit_pgm_ratios |= (bits << PFIT_HORIZ_SCALE_SHIFT |
>  							    bits << PFIT_VERT_SCALE_SHIFT);
>  					pfit_control |= (PFIT_ENABLE |
> @@ -265,8 +263,8 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  		 * Full scaling, even if it changes the aspect ratio.
>  		 * Fortunately this is all done for us in hw.
>  		 */
> -		if (mode->vdisplay != adjusted_mode->vdisplay ||
> -		    mode->hdisplay != adjusted_mode->hdisplay) {
> +		if (pipe_config->pipe_src_h != adjusted_mode->vdisplay ||
> +		    pipe_config->pipe_src_w != adjusted_mode->hdisplay) {
>  			pfit_control |= PFIT_ENABLE;
>  			if (INTEL_INFO(dev)->gen >= 4)
>  				pfit_control |= PFIT_SCALING_AUTO;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3ba412c..3823d63 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -450,9 +450,8 @@ void intel_update_fbc(struct drm_device *dev)
>  	struct drm_framebuffer *fb;
>  	struct intel_framebuffer *intel_fb;
>  	struct drm_i915_gem_object *obj;
> -	const struct drm_display_mode *mode;
>  	const struct drm_display_mode *adjusted_mode;
> -	unsigned int max_hdisplay, max_vdisplay;
> +	unsigned int max_width, max_height;
>  
>  	if (!I915_HAS_FBC(dev)) {
>  		set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
> @@ -496,7 +495,6 @@ void intel_update_fbc(struct drm_device *dev)
>  	fb = crtc->fb;
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
> -	mode = &intel_crtc->config.requested_mode;
>  	adjusted_mode = &intel_crtc->config.adjusted_mode;
>  
>  	if (i915_enable_fbc < 0 &&
> @@ -519,14 +517,14 @@ void intel_update_fbc(struct drm_device *dev)
>  	}
>  
>  	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
> -		max_hdisplay = 4096;
> -		max_vdisplay = 2048;
> +		max_width = 4096;
> +		max_height = 2048;
>  	} else {
> -		max_hdisplay = 2048;
> -		max_vdisplay = 1536;
> +		max_width = 2048;
> +		max_height = 1536;
>  	}
> -	if ((mode->hdisplay > max_hdisplay) ||
> -	    (mode->vdisplay > max_vdisplay)) {
> +	if (intel_crtc->config.pipe_src_w > max_width ||
> +	    intel_crtc->config.pipe_src_h > max_height) {
>  		if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
>  			DRM_DEBUG_KMS("mode too large for compression, disabling\n");
>  		goto out_disable;
> @@ -1177,7 +1175,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
>  	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
>  	clock = adjusted_mode->clock;
>  	htotal = adjusted_mode->htotal;
> -	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> +	hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  	/* Use the small buffer method to calculate plane watermark */
> @@ -1264,7 +1262,7 @@ static bool g4x_compute_srwm(struct drm_device *dev,
>  	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
>  	clock = adjusted_mode->clock;
>  	htotal = adjusted_mode->htotal;
> -	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> +	hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  	line_time_us = (htotal * 1000) / clock;
> @@ -1492,7 +1490,7 @@ static void i965_update_wm(struct drm_device *dev)
>  			&to_intel_crtc(crtc)->config.adjusted_mode;
>  		int clock = adjusted_mode->clock;
>  		int htotal = adjusted_mode->htotal;
> -		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> +		int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
>  		int pixel_size = crtc->fb->bits_per_pixel / 8;
>  		unsigned long line_time_us;
>  		int entries;
> @@ -1613,7 +1611,7 @@ static void i9xx_update_wm(struct drm_device *dev)
>  			&to_intel_crtc(enabled)->config.adjusted_mode;
>  		int clock = adjusted_mode->clock;
>  		int htotal = adjusted_mode->htotal;
> -		int hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> +		int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
>  		int pixel_size = enabled->fb->bits_per_pixel / 8;
>  		unsigned long line_time_us;
>  		int entries;
> @@ -1762,7 +1760,7 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
>  	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
>  	clock = adjusted_mode->clock;
>  	htotal = adjusted_mode->htotal;
> -	hdisplay = to_intel_crtc(crtc)->config.requested_mode.hdisplay;
> +	hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
>  	pixel_size = crtc->fb->bits_per_pixel / 8;
>  
>  	line_time_us = (htotal * 1000) / clock;
> @@ -2114,8 +2112,8 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
>  	if (pfit_size) {
>  		uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
>  
> -		pipe_w = intel_crtc->config.requested_mode.hdisplay;
> -		pipe_h = intel_crtc->config.requested_mode.vdisplay;
> +		pipe_w = intel_crtc->config.pipe_src_w;
> +		pipe_h = intel_crtc->config.pipe_src_h;
>  		pfit_w = (pfit_size >> 16) & 0xFFFF;
>  		pfit_h = pfit_size & 0xFFFF;
>  		if (pipe_w < pfit_w)
> @@ -2640,8 +2638,7 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
>  		p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
>  		p->pri.bytes_per_pixel = crtc->fb->bits_per_pixel / 8;
>  		p->cur.bytes_per_pixel = 4;
> -		p->pri.horiz_pixels =
> -			intel_crtc->config.requested_mode.hdisplay;
> +		p->pri.horiz_pixels = intel_crtc->config.pipe_src_w;
>  		p->cur.horiz_pixels = 64;
>  		/* TODO: for now, assume primary and cursor planes are always enabled. */
>  		p->pri.enabled = true;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 753cef3..71717e2 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -652,8 +652,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		.y2 = crtc_y + crtc_h,
>  	};
>  	const struct drm_rect clip = {
> -		.x2 = intel_crtc->config.requested_mode.hdisplay,
> -		.y2 = intel_crtc->config.requested_mode.vdisplay,
> +		.x2 = intel_crtc->config.pipe_src_w,
> +		.y2 = intel_crtc->config.pipe_src_h,
>  	};
>  
>  	intel_fb = to_intel_framebuffer(fb);
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 12/14] drm/i915: Document the inteded use of requested_mode
  2013-09-04 15:25 ` [PATCH 12/14] drm/i915: Document the inteded use of requested_mode ville.syrjala
@ 2013-09-10 17:07   ` Damien Lespiau
  2013-09-10 17:17     ` Ville Syrjälä
  2013-09-10 17:10   ` Damien Lespiau
  1 sibling, 1 reply; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 17:07 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:29PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Try to clarify the purpose of requested_mode.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9d30909..2c9c96b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -211,6 +211,11 @@ struct intel_crtc_config {
>  #define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (1<<0) /* unreliable sync mode.flags */
>  	unsigned long quirks;
>  
> +	/* User requested mode, only valid as a starting point to
> +	 * compute adjusted_mode, except in the case of (S)DVO where
> +	 * it's also for the output timings of the (S)DVO chip.
> +	 * adjusted_mode will then correspond to the S(DVO) chip's
> +	 * preferred input timings. */

That looks quite weird, maybe we should introduce another mode for this, and
only this, case?

FWIW, I've been recently reminded that:

	/*
	* This is the preferred style for multi-line
	* comments in the Linux kernel source code.
	* Please use it consistently.
	*
	* Description:  A column of asterisks on the left side,
	* with beginning and ending almost-blank lines.
	*/

except in net/ and drivers/net/ where the style in this patch is used.

-- 
Damien

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

* Re: [PATCH 12/14] drm/i915: Document the inteded use of requested_mode
  2013-09-04 15:25 ` [PATCH 12/14] drm/i915: Document the inteded use of requested_mode ville.syrjala
  2013-09-10 17:07   ` Damien Lespiau
@ 2013-09-10 17:10   ` Damien Lespiau
  1 sibling, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2013-09-10 17:10 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:29PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Try to clarify the purpose of requested_mode.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

(and intended in the summary)

-- 
Damien

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

* Re: [PATCH 12/14] drm/i915: Document the inteded use of requested_mode
  2013-09-10 17:07   ` Damien Lespiau
@ 2013-09-10 17:17     ` Ville Syrjälä
  2013-09-10 17:49       ` Daniel Vetter
  0 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2013-09-10 17:17 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Tue, Sep 10, 2013 at 06:07:18PM +0100, Damien Lespiau wrote:
> On Wed, Sep 04, 2013 at 06:25:29PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Try to clarify the purpose of requested_mode.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 9d30909..2c9c96b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -211,6 +211,11 @@ struct intel_crtc_config {
> >  #define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (1<<0) /* unreliable sync mode.flags */
> >  	unsigned long quirks;
> >  
> > +	/* User requested mode, only valid as a starting point to
> > +	 * compute adjusted_mode, except in the case of (S)DVO where
> > +	 * it's also for the output timings of the (S)DVO chip.
> > +	 * adjusted_mode will then correspond to the S(DVO) chip's
> > +	 * preferred input timings. */
> 
> That looks quite weird, maybe we should introduce another mode for this, and
> only this, case?

I was thinking about it, but then decided that I don't care enough
about it to make another patch. Maybe if I ever manage to get some SDVO
cards I might care, or if we simply decide to kill requested_mode (ie.
we'd just rename it to sdvo_output_mode or something).

> 
> FWIW, I've been recently reminded that:
> 
> 	/*
> 	* This is the preferred style for multi-line
> 	* comments in the Linux kernel source code.
> 	* Please use it consistently.
> 	*
> 	* Description:  A column of asterisks on the left side,
> 	* with beginning and ending almost-blank lines.
> 	*/
> 
> except in net/ and drivers/net/ where the style in this patch is used.

Yeah I tried it. But it looked funny since the other comments there
already used the wrong style. So you can blame Daniel or whoever wrote
the other comments ;)

-- 
Ville Syrjälä
Intel OTC

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

* Re: [PATCH 12/14] drm/i915: Document the inteded use of requested_mode
  2013-09-10 17:17     ` Ville Syrjälä
@ 2013-09-10 17:49       ` Daniel Vetter
  0 siblings, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2013-09-10 17:49 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, Sep 10, 2013 at 7:17 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>> FWIW, I've been recently reminded that:
>>
>>       /*
>>       * This is the preferred style for multi-line
>>       * comments in the Linux kernel source code.
>>       * Please use it consistently.
>>       *
>>       * Description:  A column of asterisks on the left side,
>>       * with beginning and ending almost-blank lines.
>>       */
>>
>> except in net/ and drivers/net/ where the style in this patch is used.
>
> Yeah I tried it. But it looked funny since the other comments there
> already used the wrong style. So you can blame Daniel or whoever wrote
> the other comments ;)

For new comments I've started to use official kernel style ... so will
look a bit funny while we transition ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [PATCH 14/14] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges
  2013-09-04 15:25 ` [PATCH 14/14] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges ville.syrjala
@ 2013-09-16 21:52   ` Daniel Vetter
  2013-09-17  9:56     ` Ville Syrjälä
  0 siblings, 1 reply; 34+ messages in thread
From: Daniel Vetter @ 2013-09-16 21:52 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Sep 04, 2013 at 06:25:31PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> First of all we should not be looking at fb->{width,height} as those do
> not tell us what the actual pipe size is. Second of all we need to use
> >= for the comparison.
> 
> So fix the comparison, and make use of the new pipe_src_{w,h} to
> determine the real pipe source dimensions.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Merged the entire series to dinq, thanks for patches&review. I've also
made a small note that we should have an igt testcase for these cursor
corner-cases somewhere ...

Cheers, Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 20f373b..44e0a84 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7000,19 +7000,16 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>  	int pipe = intel_crtc->pipe;
>  	int x = intel_crtc->cursor_x;
>  	int y = intel_crtc->cursor_y;
> -	u32 base, pos;
> +	u32 base = 0, pos = 0;
>  	bool visible;
>  
> -	pos = 0;
> -
> -	if (on && crtc->enabled && crtc->fb) {
> +	if (on)
>  		base = intel_crtc->cursor_addr;
> -		if (x > (int) crtc->fb->width)
> -			base = 0;
>  
> -		if (y > (int) crtc->fb->height)
> -			base = 0;
> -	} else
> +	if (x >= intel_crtc->config.pipe_src_w)
> +		base = 0;
> +
> +	if (y >= intel_crtc->config.pipe_src_h)
>  		base = 0;
>  
>  	if (x < 0) {
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 14/14] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges
  2013-09-16 21:52   ` Daniel Vetter
@ 2013-09-17  9:56     ` Ville Syrjälä
  0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2013-09-17  9:56 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Mon, Sep 16, 2013 at 11:52:17PM +0200, Daniel Vetter wrote:
> On Wed, Sep 04, 2013 at 06:25:31PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > First of all we should not be looking at fb->{width,height} as those do
> > not tell us what the actual pipe size is. Second of all we need to use
> > >= for the comparison.
> > 
> > So fix the comparison, and make use of the new pipe_src_{w,h} to
> > determine the real pipe source dimensions.
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Merged the entire series to dinq, thanks for patches&review. I've also
> made a small note that we should have an igt testcase for these cursor
> corner-cases somewhere ...

Which reminds me that I started to write one. Need to finish it up...

-- 
Ville Syrjälä
Intel OTC

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

end of thread, other threads:[~2013-09-17  9:56 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-04 15:25 [PATCH 0/14] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc. v2 ville.syrjala
2013-09-04 15:25 ` [PATCH v2 01/14] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode ville.syrjala
2013-09-10 14:27   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 02/14] drm/i915: Use adjusted_mode->clock in lpt_program_iclkip ville.syrjala
2013-09-10 14:41   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 03/14] drm/i915: Use adjusted_mode in HDMI 12bpc clock check ville.syrjala
2013-09-10 14:42   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 04/14] drm/i915: Use adjusted_mode in intel_update_fbc() ville.syrjala
2013-09-10 14:45   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 05/14] drm/i915: Use adjusted_mode appropriately when computing watermarks ville.syrjala
2013-09-10 15:00   ` Damien Lespiau
2013-09-10 15:04     ` Ville Syrjälä
2013-09-10 16:08   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 06/14] drm/i915: Check the clock from adjusted mode in intel_crtc_active() ville.syrjala
2013-09-10 15:00   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 07/14] drm/i915: Use adjusted_mode when checking conditions for PSR ville.syrjala
2013-09-10 16:42   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH v2 08/14] drm/i915: Make intel_crtc_active() available outside intel_pm.c ville.syrjala
2013-09-10 16:43   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 09/14] drm/i915: Use pipe config in sprite code ville.syrjala
2013-09-10 16:44   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 10/14] drm/i915: Use adjusted_mode in DSI PLL calculations ville.syrjala
2013-09-10 16:45   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH v2 11/14] drm/i915: Add explicit pipe src size to pipe config ville.syrjala
2013-09-10 17:02   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 12/14] drm/i915: Document the inteded use of requested_mode ville.syrjala
2013-09-10 17:07   ` Damien Lespiau
2013-09-10 17:17     ` Ville Syrjälä
2013-09-10 17:49       ` Daniel Vetter
2013-09-10 17:10   ` Damien Lespiau
2013-09-04 15:25 ` [PATCH 13/14] drm/i915: Fix cursor visibility check with negative coordinates ville.syrjala
2013-09-04 15:25 ` [PATCH 14/14] drm/i915: Fix cursor visibility checks also for the right/bottom screen edges ville.syrjala
2013-09-16 21:52   ` Daniel Vetter
2013-09-17  9:56     ` Ville Syrjälä

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