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

[Ander Conselvan de Oliveira <conselvan2@gmail.com>]

On 16-12-2014 11:34, Daniel Vetter wrote:
> 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.

I should improve the commit message. I agree this patch is a step 
backwards, but the idea is to move stuff out of the way so we can  move 
forwards again. The ultimate goal is to pass a drm_atomic_state struct, 
and derive the pipe_config from that.

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