Message ID | 20200213140412.32697-3-stanislav.lisovskiy@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Fix modeset transitions related to DBuf | expand |
On Thu, Feb 13, 2020 at 04:04:11PM +0200, Stanislav Lisovskiy wrote: > From: Jani Nikula <jani.nikula@intel.com> > > We lack full state readout of DSC config, which may lead to DSC enable > using a config that's all zeros, failing spectacularly. Force full > modeset and thus compute config at probe to get a sane state, until we > implement DSC state readout. Any fastset that did appear to work with > DSC at probe, worked by coincidence. [1] is an example of a change that > triggered the issue on TGL DSI DSC. > > [1] http://patchwork.freedesktop.org/patch/msgid/20200212150102.7600-1-ville.syrjala@linux.intel.com > > Cc: Manasi Navare <manasi.d.navare@intel.com> > Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: stable@vger.kernel.org > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Should this be Fixes: fbacb15ea814 ("drm/i915/dsc: add basic hardware state readout support") since that's where we added the basic readout with a FIXME to add more later? I don't know the specifics of DSC and what state we need, but the approach here seems reasonable. Acked-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 61ba1f2256a0..1e3f2cc27db8 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -17828,6 +17828,24 @@ static int intel_initial_commit(struct drm_device *dev) > * have readout for pipe gamma enable. > */ > crtc_state->uapi.color_mgmt_changed = true; > + > + /* > + * FIXME hack to force full modeset when DSC is being > + * used. > + * > + * As long as we do not have full state readout and > + * config comparison of crtc_state->dsc, we have no way > + * to ensure reliable fastset. Remove once we have > + * readout for DSC. > + */ > + if (crtc_state->dsc.compression_enable) { > + ret = drm_atomic_add_affected_connectors(state, > + &crtc->base); > + if (ret) > + goto out; > + crtc_state->uapi.mode_changed = true; > + drm_dbg_kms(dev, "Force full modeset for DSC\n"); > + } > } > } > > -- > 2.24.1.485.gad05a3d8e5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 13 Feb 2020, Matt Roper <matthew.d.roper@intel.com> wrote: > On Thu, Feb 13, 2020 at 04:04:11PM +0200, Stanislav Lisovskiy wrote: >> From: Jani Nikula <jani.nikula@intel.com> >> >> We lack full state readout of DSC config, which may lead to DSC enable >> using a config that's all zeros, failing spectacularly. Force full >> modeset and thus compute config at probe to get a sane state, until we >> implement DSC state readout. Any fastset that did appear to work with >> DSC at probe, worked by coincidence. [1] is an example of a change that >> triggered the issue on TGL DSI DSC. >> >> [1] http://patchwork.freedesktop.org/patch/msgid/20200212150102.7600-1-ville.syrjala@linux.intel.com >> >> Cc: Manasi Navare <manasi.d.navare@intel.com> >> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: stable@vger.kernel.org >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > Should this be > > Fixes: fbacb15ea814 ("drm/i915/dsc: add basic hardware state readout support") > > since that's where we added the basic readout with a FIXME to add more > later? That added some readout and checks, but the problem existed before them too. BR, Jani. > > I don't know the specifics of DSC and what state we need, but the > approach here seems reasonable. > > Acked-by: Matt Roper <matthew.d.roper@intel.com> > >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index 61ba1f2256a0..1e3f2cc27db8 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -17828,6 +17828,24 @@ static int intel_initial_commit(struct drm_device *dev) >> * have readout for pipe gamma enable. >> */ >> crtc_state->uapi.color_mgmt_changed = true; >> + >> + /* >> + * FIXME hack to force full modeset when DSC is being >> + * used. >> + * >> + * As long as we do not have full state readout and >> + * config comparison of crtc_state->dsc, we have no way >> + * to ensure reliable fastset. Remove once we have >> + * readout for DSC. >> + */ >> + if (crtc_state->dsc.compression_enable) { >> + ret = drm_atomic_add_affected_connectors(state, >> + &crtc->base); >> + if (ret) >> + goto out; >> + crtc_state->uapi.mode_changed = true; >> + drm_dbg_kms(dev, "Force full modeset for DSC\n"); >> + } >> } >> } >> >> -- >> 2.24.1.485.gad05a3d8e5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Feb 13, 2020 at 11:03:29PM +0200, Jani Nikula wrote: > On Thu, 13 Feb 2020, Matt Roper <matthew.d.roper@intel.com> wrote: > > On Thu, Feb 13, 2020 at 04:04:11PM +0200, Stanislav Lisovskiy wrote: > >> From: Jani Nikula <jani.nikula@intel.com> > >> > >> We lack full state readout of DSC config, which may lead to DSC enable > >> using a config that's all zeros, failing spectacularly. Force full > >> modeset and thus compute config at probe to get a sane state, until we > >> implement DSC state readout. Any fastset that did appear to work with > >> DSC at probe, worked by coincidence. [1] is an example of a change that > >> triggered the issue on TGL DSI DSC. > >> > >> [1] http://patchwork.freedesktop.org/patch/msgid/20200212150102.7600-1-ville.syrjala@linux.intel.com > >> > >> Cc: Manasi Navare <manasi.d.navare@intel.com> > >> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> > >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > >> Cc: stable@vger.kernel.org > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > > > Should this be > > > > Fixes: fbacb15ea814 ("drm/i915/dsc: add basic hardware state readout support") > > > > since that's where we added the basic readout with a FIXME to add more > > later? > > That added some readout and checks, but the problem existed before them > too. Right, but that's as far back as we can go with a fix, right? Since before that we don't even read out enough to know that compression is enabled? Matt > > BR, > Jani. > > > > > > > I don't know the specifics of DSC and what state we need, but the > > approach here seems reasonable. > > > > Acked-by: Matt Roper <matthew.d.roper@intel.com> > > > >> --- > >> drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++++++++++++++ > >> 1 file changed, 18 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > >> index 61ba1f2256a0..1e3f2cc27db8 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display.c > >> +++ b/drivers/gpu/drm/i915/display/intel_display.c > >> @@ -17828,6 +17828,24 @@ static int intel_initial_commit(struct drm_device *dev) > >> * have readout for pipe gamma enable. > >> */ > >> crtc_state->uapi.color_mgmt_changed = true; > >> + > >> + /* > >> + * FIXME hack to force full modeset when DSC is being > >> + * used. > >> + * > >> + * As long as we do not have full state readout and > >> + * config comparison of crtc_state->dsc, we have no way > >> + * to ensure reliable fastset. Remove once we have > >> + * readout for DSC. > >> + */ > >> + if (crtc_state->dsc.compression_enable) { > >> + ret = drm_atomic_add_affected_connectors(state, > >> + &crtc->base); > >> + if (ret) > >> + goto out; > >> + crtc_state->uapi.mode_changed = true; > >> + drm_dbg_kms(dev, "Force full modeset for DSC\n"); > >> + } > >> } > >> } > >> > >> -- > >> 2.24.1.485.gad05a3d8e5 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Graphics Center
On Thu, 13 Feb 2020, Matt Roper <matthew.d.roper@intel.com> wrote: > On Thu, Feb 13, 2020 at 11:03:29PM +0200, Jani Nikula wrote: >> On Thu, 13 Feb 2020, Matt Roper <matthew.d.roper@intel.com> wrote: >> > On Thu, Feb 13, 2020 at 04:04:11PM +0200, Stanislav Lisovskiy wrote: >> >> From: Jani Nikula <jani.nikula@intel.com> >> >> >> >> We lack full state readout of DSC config, which may lead to DSC enable >> >> using a config that's all zeros, failing spectacularly. Force full >> >> modeset and thus compute config at probe to get a sane state, until we >> >> implement DSC state readout. Any fastset that did appear to work with >> >> DSC at probe, worked by coincidence. [1] is an example of a change that >> >> triggered the issue on TGL DSI DSC. >> >> >> >> [1] http://patchwork.freedesktop.org/patch/msgid/20200212150102.7600-1-ville.syrjala@linux.intel.com >> >> >> >> Cc: Manasi Navare <manasi.d.navare@intel.com> >> >> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> >> Cc: stable@vger.kernel.org >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> > >> > Should this be >> > >> > Fixes: fbacb15ea814 ("drm/i915/dsc: add basic hardware state readout support") >> > >> > since that's where we added the basic readout with a FIXME to add more >> > later? >> >> That added some readout and checks, but the problem existed before them >> too. > > Right, but that's as far back as we can go with a fix, right? Since > before that we don't even read out enough to know that compression is > enabled? Ah, that's right. Any fixes predating that commit would have to be different. Perhaps we'll just settle for this. ;) Thanks, Jani. > > > Matt > >> >> BR, >> Jani. >> >> >> >> > >> > I don't know the specifics of DSC and what state we need, but the >> > approach here seems reasonable. >> > >> > Acked-by: Matt Roper <matthew.d.roper@intel.com> >> > >> >> --- >> >> drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++++++++++++++ >> >> 1 file changed, 18 insertions(+) >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> >> index 61ba1f2256a0..1e3f2cc27db8 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> >> @@ -17828,6 +17828,24 @@ static int intel_initial_commit(struct drm_device *dev) >> >> * have readout for pipe gamma enable. >> >> */ >> >> crtc_state->uapi.color_mgmt_changed = true; >> >> + >> >> + /* >> >> + * FIXME hack to force full modeset when DSC is being >> >> + * used. >> >> + * >> >> + * As long as we do not have full state readout and >> >> + * config comparison of crtc_state->dsc, we have no way >> >> + * to ensure reliable fastset. Remove once we have >> >> + * readout for DSC. >> >> + */ >> >> + if (crtc_state->dsc.compression_enable) { >> >> + ret = drm_atomic_add_affected_connectors(state, >> >> + &crtc->base); >> >> + if (ret) >> >> + goto out; >> >> + crtc_state->uapi.mode_changed = true; >> >> + drm_dbg_kms(dev, "Force full modeset for DSC\n"); >> >> + } >> >> } >> >> } >> >> >> >> -- >> >> 2.24.1.485.gad05a3d8e5 >> >> >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@lists.freedesktop.org >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 61ba1f2256a0..1e3f2cc27db8 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -17828,6 +17828,24 @@ static int intel_initial_commit(struct drm_device *dev) * have readout for pipe gamma enable. */ crtc_state->uapi.color_mgmt_changed = true; + + /* + * FIXME hack to force full modeset when DSC is being + * used. + * + * As long as we do not have full state readout and + * config comparison of crtc_state->dsc, we have no way + * to ensure reliable fastset. Remove once we have + * readout for DSC. + */ + if (crtc_state->dsc.compression_enable) { + ret = drm_atomic_add_affected_connectors(state, + &crtc->base); + if (ret) + goto out; + crtc_state->uapi.mode_changed = true; + drm_dbg_kms(dev, "Force full modeset for DSC\n"); + } } }