[PATCH 00/12] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc.

[PATCH 00/12] drm/i915: pipe_config, adjusted_mode vs. requested_mode, etc.

[ville.syrjala]

Currently parts of our code are really confused about which mode
structure they should be looking at.

A lot of the code never got converted to look at the pipe config modes,
and for example the watermark code still managed to look at the wrong mode
(crtc->mode, not even crtc->hwmode which would have been a bit better).

So let's convert everything over to pipe config, and let's pretend the
modes in drm_crtc don't even exist.

Also in the pipe config conversion some bits of code were made to look at
the requested_mode. If we have a fixed mode panel, requested_mode may contain
all kinds of garbage and the only two things in it we should be looking at
are hdisplay and vdisplay. Everything else should be interested in the
adjusted_mode only.

(S)DVO is a minor exception here as is wants to program the output timings
with the requested_mode. I've left that stuff alone for now since it's
essentially correct as is.

In the end I decided that digging out hdisplay/vdisplay from requested_mode
is just confusing. Instead I added explicit pipe source width/height members
to pipe config. I was also considering adding primary plane size to make it
clear which one we actually want in different places. But I opted not to do
that. It should get sorted out when we move to drm_planes. After this there
is no real need to poke around requested_mode anymore, so we could even consider
removing it, or rather rename it to indicate that it's only relevant for
(S)DVO.

I also found some bugs in LVDS adjusted_mode vs. fixed_mode handling. It was
causing the pipe config sanity checks to fail on my 855. So I ended up
fixing that stuff as well.

One extra idea that occured to me, is that we never actually update
adjusted_mode->clock with the real clock value after we've found the PLL settings.
I think that's something we should be doing since the PLL might not be
able to give an exact match, and we use the clock to compute watermarks and
check various hardware limits. But that's a patch for another day.

[PATCH 01/12] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode

[ville.syrjala]

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

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

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ecb8b52..cab1319 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4124,8 +4124,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 
 	if (HAS_PCH_SPLIT(dev)) {
 		/* FDI link clock is fixed at 2.7G */
-		if (pipe_config->requested_mode.clock * 3
-		    > IRONLAKE_FDI_FREQ * 4)
+		if (adjusted_mode->clock * 3 > IRONLAKE_FDI_FREQ * 4)
 			return -EINVAL;
 	}
 
@@ -4812,7 +4811,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

Re: [PATCH 01/12] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode

[Daniel Vetter]

On Mon, Sep 02, 2013 at 09:13:28PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_crtc_compute_config() and i9xx_set_pipeconf() attempt to get
> the current pixel clock from requested_mode. requested_mode.clock may
> be totally bogus, so the clock should come from adjusted_mode.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ecb8b52..cab1319 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4124,8 +4124,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  
>  	if (HAS_PCH_SPLIT(dev)) {
>  		/* FDI link clock is fixed at 2.7G */
> -		if (pipe_config->requested_mode.clock * 3
> -		    > IRONLAKE_FDI_FREQ * 4)
> +		if (adjusted_mode->clock * 3 > IRONLAKE_FDI_FREQ * 4)

Note quite: The fdi dotclock is the adjusted mode's clock but with the
pixel multiplier _not_ taken into account. See
ironlake_fdi_compute_config. Maybe we need a fdi_dotclock_from_pipe_config
helper function?o

Otoh the ironlake_fdi_compute_config will do the right thing no matter
what and even takes into account things like cpu edp or shared fdi b/c
lanes. So I think we should just ditch this check here ;-)
-Daniel

>  			return -EINVAL;
>  	}
>  
> @@ -4812,7 +4811,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

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

Re: [PATCH 01/12] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode

[Ville Syrjälä]

On Mon, Sep 02, 2013 at 08:38:16PM +0200, Daniel Vetter wrote:
> On Mon, Sep 02, 2013 at 09:13:28PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > intel_crtc_compute_config() and i9xx_set_pipeconf() attempt to get
> > the current pixel clock from requested_mode. requested_mode.clock may
> > be totally bogus, so the clock should come from adjusted_mode.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ecb8b52..cab1319 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4124,8 +4124,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >  
> >  	if (HAS_PCH_SPLIT(dev)) {
> >  		/* FDI link clock is fixed at 2.7G */
> > -		if (pipe_config->requested_mode.clock * 3
> > -		    > IRONLAKE_FDI_FREQ * 4)
> > +		if (adjusted_mode->clock * 3 > IRONLAKE_FDI_FREQ * 4)
> 
> Note quite: The fdi dotclock is the adjusted mode's clock but with the
> pixel multiplier _not_ taken into account. See
> ironlake_fdi_compute_config. Maybe we need a fdi_dotclock_from_pipe_config
> helper function?

Dang those pixel multipliers. I need to study on the topic a bit more.
I'm confused whether the pipe is actually pushing out pixels at the
non-multiplied rate or the multiplied rate. That's an important detail
when we consider the CDCLK vs. pipe pixel rate limitations.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 01/12] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode

[Daniel Vetter]

On Tue, Sep 03, 2013 at 01:01:14PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 02, 2013 at 08:38:16PM +0200, Daniel Vetter wrote:
> > On Mon, Sep 02, 2013 at 09:13:28PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > intel_crtc_compute_config() and i9xx_set_pipeconf() attempt to get
> > > the current pixel clock from requested_mode. requested_mode.clock may
> > > be totally bogus, so the clock should come from adjusted_mode.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index ecb8b52..cab1319 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -4124,8 +4124,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> > >  
> > >  	if (HAS_PCH_SPLIT(dev)) {
> > >  		/* FDI link clock is fixed at 2.7G */
> > > -		if (pipe_config->requested_mode.clock * 3
> > > -		    > IRONLAKE_FDI_FREQ * 4)
> > > +		if (adjusted_mode->clock * 3 > IRONLAKE_FDI_FREQ * 4)
> > 
> > Note quite: The fdi dotclock is the adjusted mode's clock but with the
> > pixel multiplier _not_ taken into account. See
> > ironlake_fdi_compute_config. Maybe we need a fdi_dotclock_from_pipe_config
> > helper function?
> 
> Dang those pixel multipliers. I need to study on the topic a bit more.
> I'm confused whether the pipe is actually pushing out pixels at the
> non-multiplied rate or the multiplied rate. That's an important detail
> when we consider the CDCLK vs. pipe pixel rate limitations.

On pch ports the pixel multiplier is in the pch dpll, so I think the data
pushed over the fdi link isn't multiplied. Iirc I've even bothered with
some tests, but not sure any more ... I vaguely remember that I've broken
Chris' ilk+sdvo machine a few times in the process of getting this fleshed
out ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 01/12] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode

[Ville Syrjälä]

On Tue, Sep 03, 2013 at 01:23:23PM +0200, Daniel Vetter wrote:
> On Tue, Sep 03, 2013 at 01:01:14PM +0300, Ville Syrjälä wrote:
> > On Mon, Sep 02, 2013 at 08:38:16PM +0200, Daniel Vetter wrote:
> > > On Mon, Sep 02, 2013 at 09:13:28PM +0300, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > intel_crtc_compute_config() and i9xx_set_pipeconf() attempt to get
> > > > the current pixel clock from requested_mode. requested_mode.clock may
> > > > be totally bogus, so the clock should come from adjusted_mode.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 5 ++---
> > > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index ecb8b52..cab1319 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -4124,8 +4124,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> > > >  
> > > >  	if (HAS_PCH_SPLIT(dev)) {
> > > >  		/* FDI link clock is fixed at 2.7G */
> > > > -		if (pipe_config->requested_mode.clock * 3
> > > > -		    > IRONLAKE_FDI_FREQ * 4)
> > > > +		if (adjusted_mode->clock * 3 > IRONLAKE_FDI_FREQ * 4)
> > > 
> > > Note quite: The fdi dotclock is the adjusted mode's clock but with the
> > > pixel multiplier _not_ taken into account. See
> > > ironlake_fdi_compute_config. Maybe we need a fdi_dotclock_from_pipe_config
> > > helper function?
> > 
> > Dang those pixel multipliers. I need to study on the topic a bit more.
> > I'm confused whether the pipe is actually pushing out pixels at the
> > non-multiplied rate or the multiplied rate. That's an important detail
> > when we consider the CDCLK vs. pipe pixel rate limitations.
> 
> On pch ports the pixel multiplier is in the pch dpll, so I think the data
> pushed over the fdi link isn't multiplied. Iirc I've even bothered with
> some tests, but not sure any more ... I vaguely remember that I've broken
> Chris' ilk+sdvo machine a few times in the process of getting this fleshed
> out ;-)

Right, so at least on SDVO the multiplier is added to keep the SDVO
clock on the above 100 MHz. So the multiplier won't actually affect
the pixel clock.

So could we just make port_clock be the multiplied clock for SDVO
and make adjusted_mode.clock be the actual pixel clock?

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 01/12] drm/i915: Grab the pixel clock from adjusted_mode not requested_mode

[Daniel Vetter]

On Tue, Sep 03, 2013 at 03:08:50PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 03, 2013 at 01:23:23PM +0200, Daniel Vetter wrote:
> > On Tue, Sep 03, 2013 at 01:01:14PM +0300, Ville Syrjälä wrote:
> > > On Mon, Sep 02, 2013 at 08:38:16PM +0200, Daniel Vetter wrote:
> > > > On Mon, Sep 02, 2013 at 09:13:28PM +0300, ville.syrjala@linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > intel_crtc_compute_config() and i9xx_set_pipeconf() attempt to get
> > > > > the current pixel clock from requested_mode. requested_mode.clock may
> > > > > be totally bogus, so the clock should come from adjusted_mode.
> > > > > 
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_display.c | 5 ++---
> > > > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > > index ecb8b52..cab1319 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -4124,8 +4124,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> > > > >  
> > > > >  	if (HAS_PCH_SPLIT(dev)) {
> > > > >  		/* FDI link clock is fixed at 2.7G */
> > > > > -		if (pipe_config->requested_mode.clock * 3
> > > > > -		    > IRONLAKE_FDI_FREQ * 4)
> > > > > +		if (adjusted_mode->clock * 3 > IRONLAKE_FDI_FREQ * 4)
> > > > 
> > > > Note quite: The fdi dotclock is the adjusted mode's clock but with the
> > > > pixel multiplier _not_ taken into account. See
> > > > ironlake_fdi_compute_config. Maybe we need a fdi_dotclock_from_pipe_config
> > > > helper function?
> > > 
> > > Dang those pixel multipliers. I need to study on the topic a bit more.
> > > I'm confused whether the pipe is actually pushing out pixels at the
> > > non-multiplied rate or the multiplied rate. That's an important detail
> > > when we consider the CDCLK vs. pipe pixel rate limitations.
> > 
> > On pch ports the pixel multiplier is in the pch dpll, so I think the data
> > pushed over the fdi link isn't multiplied. Iirc I've even bothered with
> > some tests, but not sure any more ... I vaguely remember that I've broken
> > Chris' ilk+sdvo machine a few times in the process of getting this fleshed
> > out ;-)
> 
> Right, so at least on SDVO the multiplier is added to keep the SDVO
> clock on the above 100 MHz. So the multiplier won't actually affect
> the pixel clock.
> 
> So could we just make port_clock be the multiplied clock for SDVO
> and make adjusted_mode.clock be the actual pixel clock?

There's also the pixel multiplier thing for hdmi and I'm not sure how it
works there. But generally I think using the port clock with the pixel
multiplier but excluding it from the adjusted mode is a sensible approach.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: Kill IRONLAKE_FDI_FREQ check

[ville.syrjala]

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

ironlake_fdi_compute_config() already checks that we have enough
FDI bandwidth. And it doesn't just use a hardcoded value but takes
into account factors such as the actual FDI frequency, shared FDI
B/C lanes, etc.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ecb8b52..8808b17 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -69,9 +69,6 @@ struct intel_limit {
 	intel_p2_t	    p2;
 };
 
-/* FDI */
-#define IRONLAKE_FDI_FREQ		2700000 /* in kHz for mode->clock */
-
 int
 intel_pch_rawclk(struct drm_device *dev)
 {
@@ -4122,13 +4119,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
 
-	if (HAS_PCH_SPLIT(dev)) {
-		/* FDI link clock is fixed at 2.7G */
-		if (pipe_config->requested_mode.clock * 3
-		    > IRONLAKE_FDI_FREQ * 4)
-			return -EINVAL;
-	}
-
 	/* Cantiga+ cannot handle modes with a hsync front porch of 0.
 	 * WaPruneModeWithIncorrectHsyncOffset:ctg,elk,ilk,snb,ivb,vlv,hsw.
 	 */
-- 
1.8.1.5

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

Re: [PATCH] drm/i915: Kill IRONLAKE_FDI_FREQ check

[Daniel Vetter]

On Tue, Sep 03, 2013 at 01:31:38PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> ironlake_fdi_compute_config() already checks that we have enough
> FDI bandwidth. And it doesn't just use a hardcoded value but takes
> into account factors such as the actual FDI frequency, shared FDI
> B/C lanes, etc.
> 
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

[PATCH 02/12] drm/i915: Use adjusted_mode->clock in lpt_program_iclkip

[ville.syrjala]

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 cab1319..19b203c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2893,6 +2893,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;
 
@@ -2910,13 +2911,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.
@@ -2925,7 +2926,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;
 
@@ -2941,7 +2942,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

[PATCH 03/12] drm/i915: Use adjusted_mode in HDMI 12bpc clock check

[ville.syrjala]

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 4148cc8..48e3b32 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -862,7 +862,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

Re: [PATCH 03/12] drm/i915: Use adjusted_mode in HDMI 12bpc clock check

[Daniel Vetter]

On Mon, Sep 02, 2013 at 09:13:30PM +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>

Atm only the encoder overrides the adjusted_mode at all, so I think I
prefer the current code flow as clearer ...
-Daniel

> ---
>  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 4148cc8..48e3b32 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -862,7 +862,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

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

Re: [PATCH 03/12] drm/i915: Use adjusted_mode in HDMI 12bpc clock check

[Ville Syrjälä]

On Mon, Sep 02, 2013 at 08:39:45PM +0200, Daniel Vetter wrote:
> On Mon, Sep 02, 2013 at 09:13:30PM +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>
> 
> Atm only the encoder overrides the adjusted_mode at all, so I think I
> prefer the current code flow as clearer ...

And I'd atually prefer to kill requested_mode. It's confusing.

Only hdisplay/vdisplay are always valid. Well even that's not true
as can be seen from my double wide series.

The clock and timings can be trusted only at the very beginning of the
compute config step. And at that point adjusted_mode contains the exact
same information. Also apart from the clock, we never use the other
timings from requested_mode, so keeping the whole thing around seems
more or less pointless.

> -Daniel
> 
> > ---
> >  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 4148cc8..48e3b32 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -862,7 +862,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
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 03/12] drm/i915: Use adjusted_mode in HDMI 12bpc clock check

[Daniel Vetter]

On Mon, Sep 02, 2013 at 10:05:26PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 02, 2013 at 08:39:45PM +0200, Daniel Vetter wrote:
> > On Mon, Sep 02, 2013 at 09:13:30PM +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>
> > 
> > Atm only the encoder overrides the adjusted_mode at all, so I think I
> > prefer the current code flow as clearer ...
> 
> And I'd atually prefer to kill requested_mode. It's confusing.
> 
> Only hdisplay/vdisplay are always valid. Well even that's not true
> as can be seen from my double wide series.
> 
> The clock and timings can be trusted only at the very beginning of the
> compute config step. And at that point adjusted_mode contains the exact
> same information. Also apart from the clock, we never use the other
> timings from requested_mode, so keeping the whole thing around seems
> more or less pointless.

Ok, count me slightly convinced. I agree that after the pipe config
compute stage the only thing we can rely on is h/vdisplay (it's also the
only thing we actually cross-check). So I guess ditching
config->requested_mode is a good goal. If you throw a patch on top to add
a bit of documentation for the adjusted_mode and the pipe_src_h|w fields
I'll even be happy ;-)

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

[PATCH] drm/i915: Document the use of requested_mode, adjusted_mode, and pipe_src_{w, h}

[ville.syrjala]

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

Try to clarify the purpose of the two different modes we keep, and the
pipe source size.

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

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a71b47c..75e86da 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -211,9 +211,19 @@ 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. */
 	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
-- 
1.8.1.5

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

[PATCH 04/12] drm/i915: Use adjusted_mode in intel_update_fbc()

[ville.syrjala]

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

[PATCH 05/12] drm/i915: Use adjusted_mode appropriately when computing watermarks

[ville.syrjala]

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

[PATCH 06/12] drm/i915: Check the clock from adjusted mode in intel_crtc_active()

[ville.syrjala]

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

[PATCH 07/12] drm/i915: Use adjusted_mode when checking conditions for PSR

[ville.syrjala]

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 2151d13..e2cb650 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1555,7 +1555,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;
@@ -1582,7 +1583,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

[PATCH 08/12] drm/i915: Make intel_crtc_active() available outside intel_pm.c

[ville.syrjala]

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().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
 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, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 19b203c..f49dbe8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -751,6 +751,17 @@ 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.
+	 */
+	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 e2cb650..5ce5968 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1555,8 +1555,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 5efb844..e017c30 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -798,5 +798,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

Re: [PATCH 08/12] drm/i915: Make intel_crtc_active() available outside intel_pm.c

[Daniel Vetter]

On Mon, Sep 02, 2013 at 09:13:35PM +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().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
>  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, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 19b203c..f49dbe8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -751,6 +751,17 @@ 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.
> +	 */
> +	return intel_crtc->active && crtc->fb &&
> +		intel_crtc->config.adjusted_mode.clock;

If you touch this can you please add a big comment explaining that we can
ditch the adjusted_mode.clock check as soon as Haswell has gained clock
readout/fastboot support and that we can ditch the crtc->fb check as soon
as we can properly reconstruct framebuffers?

Thanks, Daniel

> +}
> +
>  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 e2cb650..5ce5968 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1555,8 +1555,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 5efb844..e017c30 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -798,5 +798,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

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

[PATCH v2] drm/i915: Make intel_crtc_active() available outside intel_pm.c

[ville.syrjala]

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 19b203c..265b813 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -751,6 +751,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 e2cb650..5ce5968 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1555,8 +1555,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 5efb844..e017c30 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -798,5 +798,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

[PATCH 09/12] drm/i915: Use pipe config in sprite code

[ville.syrjala]

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

[PATCH 10/12] drm/i915: Use adjusted_mode in DSI PLL calculations

[ville.syrjala]

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

[PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

[ville.syrjala]

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.

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     |  3 ++
 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, 64 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f49dbe8..17fe7ed 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4692,7 +4692,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
@@ -4745,7 +4744,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,
@@ -4783,8 +4783,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,
@@ -4880,7 +4883,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;
@@ -4978,8 +4980,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);
@@ -8255,6 +8257,8 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	drm_mode_debug_printmodeline(&pipe_config->requested_mode);
 	DRM_DEBUG_KMS("adjusted mode:\n");
 	drm_mode_debug_printmodeline(&pipe_config->adjusted_mode);
+	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,
@@ -8306,6 +8310,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;
@@ -8649,8 +8657,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 e017c30..594d9f4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -213,6 +213,9 @@ struct intel_crtc_config {
 
 	struct drm_display_mode requested_mode;
 	struct drm_display_mode adjusted_mode;
+
+	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 913cb9d..c361f04 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -60,23 +60,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;
@@ -84,17 +83,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;
@@ -186,14 +185,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) {
@@ -202,16 +200,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 */
@@ -221,12 +219,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
@@ -236,11 +234,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 |
@@ -250,11 +248,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 |
@@ -275,8 +273,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

Re: [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

[Daniel Vetter]

On Mon, Sep 02, 2013 at 09:13:38PM +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.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Not sold on this - imo it makes more sense to keep track of this in a new
plane config structure or something similar which ties the fb + metadata
to the crtc. Then we could move a bunch of things we currently have in the
pipe config or someplaces else even (fbc, ips, ...) over there.

So I'm voting for keeping the mess a bit longer until we can do the real
thing ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 ++
>  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, 64 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f49dbe8..17fe7ed 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4692,7 +4692,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
> @@ -4745,7 +4744,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,
> @@ -4783,8 +4783,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,
> @@ -4880,7 +4883,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;
> @@ -4978,8 +4980,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);
> @@ -8255,6 +8257,8 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  	drm_mode_debug_printmodeline(&pipe_config->requested_mode);
>  	DRM_DEBUG_KMS("adjusted mode:\n");
>  	drm_mode_debug_printmodeline(&pipe_config->adjusted_mode);
> +	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,
> @@ -8306,6 +8310,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;
> @@ -8649,8 +8657,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 e017c30..594d9f4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -213,6 +213,9 @@ struct intel_crtc_config {
>  
>  	struct drm_display_mode requested_mode;
>  	struct drm_display_mode adjusted_mode;
> +
> +	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 913cb9d..c361f04 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -60,23 +60,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;
> @@ -84,17 +83,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;
> @@ -186,14 +185,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) {
> @@ -202,16 +200,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 */
> @@ -221,12 +219,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
> @@ -236,11 +234,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 |
> @@ -250,11 +248,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 |
> @@ -275,8 +273,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

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

Re: [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

[Ville Syrjälä]

On Mon, Sep 02, 2013 at 08:46:19PM +0200, Daniel Vetter wrote:
> On Mon, Sep 02, 2013 at 09:13:38PM +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.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Not sold on this - imo it makes more sense to keep track of this in a new
> plane config structure or something similar which ties the fb + metadata
> to the crtc. Then we could move a bunch of things we currently have in the
> pipe config or someplaces else even (fbc, ips, ...) over there.

This size isn't the plane size. It's the pipe size. Two different things.

> 
> So I'm voting for keeping the mess a bit longer until we can do the real
> thing ...
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++------
> >  drivers/gpu/drm/i915/intel_drv.h     |  3 ++
> >  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, 64 insertions(+), 58 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f49dbe8..17fe7ed 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4692,7 +4692,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
> > @@ -4745,7 +4744,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,
> > @@ -4783,8 +4783,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,
> > @@ -4880,7 +4883,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;
> > @@ -4978,8 +4980,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);
> > @@ -8255,6 +8257,8 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> >  	drm_mode_debug_printmodeline(&pipe_config->requested_mode);
> >  	DRM_DEBUG_KMS("adjusted mode:\n");
> >  	drm_mode_debug_printmodeline(&pipe_config->adjusted_mode);
> > +	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,
> > @@ -8306,6 +8310,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;
> > @@ -8649,8 +8657,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 e017c30..594d9f4 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -213,6 +213,9 @@ struct intel_crtc_config {
> >  
> >  	struct drm_display_mode requested_mode;
> >  	struct drm_display_mode adjusted_mode;
> > +
> > +	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 913cb9d..c361f04 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -60,23 +60,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;
> > @@ -84,17 +83,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;
> > @@ -186,14 +185,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) {
> > @@ -202,16 +200,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 */
> > @@ -221,12 +219,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
> > @@ -236,11 +234,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 |
> > @@ -250,11 +248,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 |
> > @@ -275,8 +273,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
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

[Daniel Vetter]

On Mon, Sep 02, 2013 at 10:11:43PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 02, 2013 at 08:46:19PM +0200, Daniel Vetter wrote:
> > On Mon, Sep 02, 2013 at 09:13:38PM +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.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Not sold on this - imo it makes more sense to keep track of this in a new
> > plane config structure or something similar which ties the fb + metadata
> > to the crtc. Then we could move a bunch of things we currently have in the
> > pipe config or someplaces else even (fbc, ips, ...) over there.
> 
> This size isn't the plane size. It's the pipe size. Two different things.

Oops, I guess I'm confused about this. In that case this makes tons of
sense. Now just one of our residential watermarks experts (definitely not
me) needs to double-check that we're always using the right thing ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 11/12] drm/i915: Add explicit pipe src size to pipe config

[Ville Syrjälä]

On Tue, Sep 03, 2013 at 09:52:40AM +0200, Daniel Vetter wrote:
> On Mon, Sep 02, 2013 at 10:11:43PM +0300, Ville Syrjälä wrote:
> > On Mon, Sep 02, 2013 at 08:46:19PM +0200, Daniel Vetter wrote:
> > > On Mon, Sep 02, 2013 at 09:13:38PM +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.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Not sold on this - imo it makes more sense to keep track of this in a new
> > > plane config structure or something similar which ties the fb + metadata
> > > to the crtc. Then we could move a bunch of things we currently have in the
> > > pipe config or someplaces else even (fbc, ips, ...) over there.
> > 
> > This size isn't the plane size. It's the pipe size. Two different things.
> 
> Oops, I guess I'm confused about this. In that case this makes tons of
> sense. Now just one of our residential watermarks experts (definitely not
> me) needs to double-check that we're always using the right thing ...

Actually we're now using pipe_src_{w,h} always. I was considering adding
primary_{w,h} to pipe config already, but then decided that I could wait
until we split the primary from the crtc.

The reason I need to use pipe_src_{w,h} is the pipe_src_w roudning for
double wide & co. We need to clip the primary to the pipe dimensions, so
the same value works as long as primary planes are fullscreen only.

But if you'd prefer I could still add the explicit primary dimensions
to pipe config.

-- 
Ville Syrjälä
Intel OTC

[PATCH 12/12] drm/i915: Fix pipe config warnings when dealing with LVDS fixed mode

[ville.syrjala]

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

intel_fixed_panel_mode() overwrote the adjusted_mode with the fixed mode
only partially. Notably it forgot to copy over the sync flags. The LVDS
code however programmed the hardware with the sync flags from fixed
mode, and then later the pipe config comparison obviously failed as we
filled out the adjusted_mode in get_config from the real registers.

Just call drm_mode_copy() in intel_fixed_panel_mode() to copy over the
whole thing, and then just use adjusted_mode in the LVDS code to figure
out which sync settings the hardware needs.

Also constify the fixed_mode argument to intel_fixed_panel_mode().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h   |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c  |  8 ++++----
 drivers/gpu/drm/i915/intel_panel.c | 14 ++------------
 3 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 594d9f4..a71b47c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -559,7 +559,7 @@ extern int intel_panel_init(struct intel_panel *panel,
 			    struct drm_display_mode *fixed_mode);
 extern void intel_panel_fini(struct intel_panel *panel);
 
-extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
+extern void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 				   struct drm_display_mode *adjusted_mode);
 extern void intel_pch_panel_fitting(struct intel_crtc *crtc,
 				    struct intel_crtc_config *pipe_config,
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 4d33278..831a5c0 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -128,8 +128,8 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
-	struct drm_display_mode *fixed_mode =
-		lvds_encoder->attached_connector->base.panel.fixed_mode;
+	const struct drm_display_mode *adjusted_mode =
+		&crtc->config.adjusted_mode;
 	int pipe = crtc->pipe;
 	u32 temp;
 
@@ -183,9 +183,9 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
 			temp &= ~LVDS_ENABLE_DITHER;
 	}
 	temp &= ~(LVDS_HSYNC_POLARITY | LVDS_VSYNC_POLARITY);
-	if (fixed_mode->flags & DRM_MODE_FLAG_NHSYNC)
+	if (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC)
 		temp |= LVDS_HSYNC_POLARITY;
-	if (fixed_mode->flags & DRM_MODE_FLAG_NVSYNC)
+	if (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC)
 		temp |= LVDS_VSYNC_POLARITY;
 
 	I915_WRITE(lvds_encoder->reg, temp);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index c361f04..c9dba46 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -36,20 +36,10 @@
 #define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
 
 void
-intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
+intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 		       struct drm_display_mode *adjusted_mode)
 {
-	adjusted_mode->hdisplay = fixed_mode->hdisplay;
-	adjusted_mode->hsync_start = fixed_mode->hsync_start;
-	adjusted_mode->hsync_end = fixed_mode->hsync_end;
-	adjusted_mode->htotal = fixed_mode->htotal;
-
-	adjusted_mode->vdisplay = fixed_mode->vdisplay;
-	adjusted_mode->vsync_start = fixed_mode->vsync_start;
-	adjusted_mode->vsync_end = fixed_mode->vsync_end;
-	adjusted_mode->vtotal = fixed_mode->vtotal;
-
-	adjusted_mode->clock = fixed_mode->clock;
+	drm_mode_copy(adjusted_mode, fixed_mode);
 
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
 }
-- 
1.8.1.5

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

Re: [PATCH 12/12] drm/i915: Fix pipe config warnings when dealing with LVDS fixed mode

[Daniel Vetter]

On Mon, Sep 02, 2013 at 09:13:39PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_fixed_panel_mode() overwrote the adjusted_mode with the fixed mode
> only partially. Notably it forgot to copy over the sync flags. The LVDS
> code however programmed the hardware with the sync flags from fixed
> mode, and then later the pipe config comparison obviously failed as we
> filled out the adjusted_mode in get_config from the real registers.
> 
> Just call drm_mode_copy() in intel_fixed_panel_mode() to copy over the
> whole thing, and then just use adjusted_mode in the LVDS code to figure
> out which sync settings the hardware needs.
> 
> Also constify the fixed_mode argument to intel_fixed_panel_mode().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Makes too much sense, so merged right away to dinq ;-)

Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/intel_drv.h   |  2 +-
>  drivers/gpu/drm/i915/intel_lvds.c  |  8 ++++----
>  drivers/gpu/drm/i915/intel_panel.c | 14 ++------------
>  3 files changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 594d9f4..a71b47c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -559,7 +559,7 @@ extern int intel_panel_init(struct intel_panel *panel,
>  			    struct drm_display_mode *fixed_mode);
>  extern void intel_panel_fini(struct intel_panel *panel);
>  
> -extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
> +extern void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>  				   struct drm_display_mode *adjusted_mode);
>  extern void intel_pch_panel_fitting(struct intel_crtc *crtc,
>  				    struct intel_crtc_config *pipe_config,
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 4d33278..831a5c0 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -128,8 +128,8 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> -	struct drm_display_mode *fixed_mode =
> -		lvds_encoder->attached_connector->base.panel.fixed_mode;
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc->config.adjusted_mode;
>  	int pipe = crtc->pipe;
>  	u32 temp;
>  
> @@ -183,9 +183,9 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
>  			temp &= ~LVDS_ENABLE_DITHER;
>  	}
>  	temp &= ~(LVDS_HSYNC_POLARITY | LVDS_VSYNC_POLARITY);
> -	if (fixed_mode->flags & DRM_MODE_FLAG_NHSYNC)
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC)
>  		temp |= LVDS_HSYNC_POLARITY;
> -	if (fixed_mode->flags & DRM_MODE_FLAG_NVSYNC)
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC)
>  		temp |= LVDS_VSYNC_POLARITY;
>  
>  	I915_WRITE(lvds_encoder->reg, temp);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index c361f04..c9dba46 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -36,20 +36,10 @@
>  #define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
>  
>  void
> -intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
> +intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>  		       struct drm_display_mode *adjusted_mode)
>  {
> -	adjusted_mode->hdisplay = fixed_mode->hdisplay;
> -	adjusted_mode->hsync_start = fixed_mode->hsync_start;
> -	adjusted_mode->hsync_end = fixed_mode->hsync_end;
> -	adjusted_mode->htotal = fixed_mode->htotal;
> -
> -	adjusted_mode->vdisplay = fixed_mode->vdisplay;
> -	adjusted_mode->vsync_start = fixed_mode->vsync_start;
> -	adjusted_mode->vsync_end = fixed_mode->vsync_end;
> -	adjusted_mode->vtotal = fixed_mode->vtotal;
> -
> -	adjusted_mode->clock = fixed_mode->clock;
> +	drm_mode_copy(adjusted_mode, fixed_mode);
>  
>  	drm_mode_set_crtcinfo(adjusted_mode, 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