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 11:08:33AM +0200, Maarten Lankhorst wrote:
> Op 08-08-16 om 10:57 schreef Ville Syrjälä:
> > 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.
> >
> The first flag is used by the watermark sanitization, second flag seems to be useful to make the driver init faster, first commit (either by fbcon or userspace) will incur the real penalty. This is similar to fastset, which is also recalculated on the first modeset.

The skl flag looks to be about the DDB. Irrelevant on other platforms,
except gen2-4 if/when someone decides to implement DDB reallocation
for them.

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