Re: [PATCH] drm/i915: Check for I915_MODE_FLAG_INHERITED before drm_atomic_helper_check_modeset

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 01 Mar 2018, Lyude Paul <lyude@redhat.com> wrote:
> Pushed with some small whitespace changes to make sparse happy, thanks!

Please do not push patches before they've passed CI. This patch gives [1]:

[  281.167033] i915 0000:00:02.0: DP-2: EDID is invalid:
...
[  282.806393] [drm:intel_enable_shared_dpll [i915]] *ERROR* DPLL 1 not locked

I don't know if this is caused by the patch, but since we get this in
the BAT round, the full IGT testing wasn't even run here.


BR,
Jani.


[1] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8102/fi-skl-6700k2/igt@kms_chamelium@common-hpd-after-suspend.html
>
> On Wed, 2018-02-21 at 10:28 +0100, Maarten Lankhorst wrote:
>> Moving the check upwards will mean we we no longer have to add planes
>> and connectors manually, because everything is handled correctly by
>> drm_atomic_helper_check_modeset() as intended.
>> 
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 20 +++++---------------
>>  1 file changed, 5 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 65be7af7f647..c5cc9022d545 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11927,6 +11927,11 @@ static int intel_atomic_check(struct drm_device
>> *dev,
>>  	int ret, i;
>>  	bool any_ms = false;
>>  
>> +	/* Catch I915_MODE_FLAG_INHERITED */
>> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
>> crtc_state, i)
>> +		if (crtc_state->mode.private_flags != old_crtc_state-
>> >mode.private_flags)
>> +			crtc_state->mode_changed = true;
>> +
>>  	ret = drm_atomic_helper_check_modeset(dev, state);
>>  	if (ret)
>>  		return ret;
>> @@ -11935,10 +11940,6 @@ static int intel_atomic_check(struct drm_device
>> *dev,
>>  		struct intel_crtc_state *pipe_config =
>>  			to_intel_crtc_state(crtc_state);
>>  
>> -		/* Catch I915_MODE_FLAG_INHERITED */
>> -		if (crtc_state->mode.private_flags != old_crtc_state-
>> >mode.private_flags)
>> -			crtc_state->mode_changed = true;
>> -
>>  		if (!needs_modeset(crtc_state))
>>  			continue;
>>  
>> @@ -11947,13 +11948,6 @@ static int intel_atomic_check(struct drm_device
>> *dev,
>>  			continue;
>>  		}
>>  
>> -		/* FIXME: For only active_changed we shouldn't need to do
>> any
>> -		 * state recomputation at all. */
>> -
>> -		ret = drm_atomic_add_affected_connectors(state, crtc);
>> -		if (ret)
>> -			return ret;
>> -
>>  		ret = intel_modeset_pipe_config(crtc, pipe_config);
>>  		if (ret) {
>>  			intel_dump_pipe_config(to_intel_crtc(crtc),
>> @@ -11972,10 +11966,6 @@ static int intel_atomic_check(struct drm_device
>> *dev,
>>  		if (needs_modeset(crtc_state))
>>  			any_ms = true;
>>  
>> -		ret = drm_atomic_add_affected_planes(state, crtc);
>> -		if (ret)
>> -			return ret;
>> -
>>  		intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
>>  				       needs_modeset(crtc_state) ?
>>  				       "[modeset]" : "[fastset]");

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