Re: [PATCH 5/8] drm/i915: Don't access to crtc->new_config from intel_mode_max_pixclk()

From: Daniel Vetter <daniel@ffwll.ch>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Ander Conselvan de Oliveira
	<ander.conselvan.de.oliveira@intel.com>,
	intel-gfx@lists.freedesktop.org, shuang.he@linux.intel.com
Subject: Re: [PATCH 5/8] drm/i915: Don't access to crtc->new_config from intel_mode_max_pixclk()
Date: Tue, 16 Dec 2014 10:34:34 +0100	[thread overview]
Message-ID: <20141216093434.GR2711@phenom.ffwll.local> (raw)
In-Reply-To: <20141215192423.GH10649@intel.com>

On Mon, Dec 15, 2014 at 09:24:23PM +0200, Ville Syrjälä wrote:
> On Thu, Dec 11, 2014 at 02:38:08PM +0200, Ander Conselvan de Oliveira wrote:
> > So that we can get rid of the new_config pointer later.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++--------
> >  1 file changed, 22 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index da5af23..a9f3034 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4897,27 +4897,39 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> >  }
> >  
> >  /* compute the max pixel clock for new configuration */
> > -static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
> > +static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv,
> > +				 struct intel_crtc *mode_set_crtc,
> > +				 struct intel_crtc_state *crtc_new_state)
> >  {
> >  	struct drm_device *dev = dev_priv->dev;
> >  	struct intel_crtc *intel_crtc;
> >  	int max_pixclk = 0;
> > +	int pixclk;
> >  
> >  	for_each_intel_crtc(dev, intel_crtc) {
> > -		if (intel_crtc->new_enabled)
> > -			max_pixclk = max(max_pixclk,
> > -					 intel_crtc->new_config->base.adjusted_mode.crtc_clock);
> > +		if (!intel_crtc->new_enabled)
> > +			continue;
> > +
> > +		if (intel_crtc == mode_set_crtc)
> > +			pixclk = crtc_new_state->base.adjusted_mode.crtc_clock;
> > +		else
> > +			pixclk = intel_crtc->config->base.adjusted_mode.crtc_clock;
> > +
> > +		max_pixclk = max(max_pixclk, pixclk);
> 
> I think we're now going backwards. We want atomic modesets, so we
> need to be able to compute this stuff when modesetting multiple pipes.
> That was precisely why I added new_config.

We've already started going backwards in this area with the latest round
of fastboot patches from Jesse. I've discussed this a bit with him and the
plan is that we'll do it that way and Ander gets shafted^W signed up to
sort out the mess.

But yeah if there's a shortcut to avoid some of that detour it would be
good.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx