[Intel-gfx] [PATCH] drm/i915: Resume DP MST before doing any kind of modesetting

[Rob Clark]

On Tue, Feb 23, 2016 at 9:33 PM, Thulasimani, Sivakumar
<sivakumar.thulasimani at intel.com> wrote:
>
>
> On 2/24/2016 3:41 AM, Lyude wrote:
>>
>> As it turns out, resuming DP MST is racey since we don't make sure MST
>> is ready before we start modesetting, it just usually happens to be
>> ready in time. This isn't the case on all systems, particularly a
>> ThinkPad T560 with displays connected through the dock. On these
>> systems, resuming the laptop while connected to the dock usually results
>> in blank monitors. Making sure MST is ready before doing any kind of
>> modesetting fixes this issue.
>
> basic question since i haven't worked on MST much. MST should work like any
> other digital panel on resume. i.e detect followed by modeset. in the
> modified
> commit mentioned below is it failing to detect the panel or failing at the
> modeset ?
> if we are depending on the intel_display_resume, how about moving the
> intel_dp_mst_resume just above intel_display_resume?
>

so I think the issue is there needs to be communication with (for
example) hubs that sit downstream..  we probably do need enough clks
and irq's going so that dpcd xfer's work, but then I think we should
do mst_resume() first and then display_resume()

BR,
-R

>
> Generic question to others in mail list on i915_drm_resume
> we are doing a modeset and then doing the detect/hpd init.
> shouldn't this be the other way round ? almost all displays
> will pass a modeset even if display is not connected so we
> are spending time on modeset even for displays that were
> removed during the suspend state. if this is to simply
> drm_state being saved and restored,
>>
>> We originally changed the resume order in
>>
>>         commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw
>> state")
>>
>> to fix a ton of WARN_ON's after resume, but this doesn't seem to be an
>> issue now anyhow.
>>
>> CC: stable at vger.kernel.org
>> Signed-off-by: Lyude <cpaul at redhat.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index f357058..4dcf3dd 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -733,6 +733,14 @@ static int i915_drm_resume(struct drm_device *dev)
>>         intel_opregion_setup(dev);
>>         intel_init_pch_refclk(dev);
>> +
>> +       /*
>> +        * We need to make sure that we resume MST before doing anything
>> +        * display related, otherwise we risk trying to bring up a display
>> on
>> +        * MST before the hub is actually ready
>> +        */
>> +       intel_dp_mst_resume(dev);
>> +
>
> This does not look proper. if the CD clock is turned off during suspend
> our dpcd read itself might fail till we enable CD Clock.
>
> regards,
> Sivakumar
>>
>>         drm_mode_config_reset(dev);
>>         /*
>> @@ -765,8 +773,6 @@ static int i915_drm_resume(struct drm_device *dev)
>>         intel_display_resume(dev);
>>         drm_modeset_unlock_all(dev);
>>   -     intel_dp_mst_resume(dev);
>> -
>>         /*
>>          * ... but also need to make sure that hotplug processing
>>          * doesn't cause havoc. Like in the driver load code we don't
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx