Re: [PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Aug 08, 2016 at 10:40:42AM +0200, Maarten Lankhorst wrote:
> Op 08-08-16 om 10:05 schreef Ville Syrjälä:
> > On Mon, Aug 08, 2016 at 09:52:49AM +0200, Maarten Lankhorst wrote:
> >> Hey,
> >>
> >> Op 05-08-16 om 22:28 schreef ville.syrjala@linux.intel.com:
> >>> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>
> >>> This function would call drm_modeset_lock_all, while the suspend/resume
> >>> functions already have their own locking. Fix this by factoring out
> >>> __intel_display_resume, and calling the atomic helpers for duplicating
> >>> atomic state and disabling all crtc's during suspend.
> >>>
> >>> Changes since v1:
> >>> - Deal with -EDEADLK right after lock_all and clean up calls
> >>>   to hw readout.
> >>> - Always take all modeset locks so updates during gpu reset are blocked.
> >>> Changes since v2:
> >>> - Fix deadlock in intel_update_primary_planes.
> >>> - Move WARN_ON(EDEADLK) to __intel_display_resume.
> >>> - pctx -> ctx
> >>> - only call __intel_display_resume on success in intel_display_resume.
> >>> Changes since v3:
> >>> - Rebase on top of dev_priv -> dev change.
> >>> - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
> >>> Changes since v4 [by vsyrjala]:
> >>> - Deal with skip_intermediate_wm
> >>> - Update comment w.r.t. mode_config.mutex vs. ->detect()
> >>> - Rebase due to INTEL_GEN() etc.
> >> Setting skip_intermediate_wm seems to have already been upstreamed and I missed it, but
> >> this may blow up in .crtc_enable, which programs in the intermediate wm's which is used
> >> until all planes are enabled.
> > What blows up and how?
> >
> > Even if it can blow up we don't have any two stage wm stuff for pre-g4x at
> > this time anyway, so -ENOCARE at this point really.
> >
> >> I fear this may blow up in interesting ways. And it should probably be using
> >> dev_priv->wm.distrust_bios_wm instead like on SKL.
> > Sigh. How many ways do we need to do the same thing?
> >
> > Anywyas, what we should really do is sanitize the current wms better
> > at readout time, and then we shouldn't need these flags at all.
> >
> Yeah, slightly different approach of accomplishing the same. :-/
> 
> distrust_bios_wm pulls in the whole state and recalculates it, while sanitize_watermarks runs at the end of initial config.
> Maybe get_hw_state for ILK should set the flag too, and  then stuff final wm in intermediate. And then kill off the skip_intermediate_wm flag.

Or just kill both flags and sanitize better.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx