Re: [PATCH 1/5] drm: Add atomic helper to redo a modeset on current mode

[Manasi Navare <manasi.d.navare@intel.com>]

On Tue, Oct 25, 2016 at 03:09:39PM +0300, Jani Nikula wrote:
> On Sat, 22 Oct 2016, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > This function provides a way for the driver to redo a
> > modeset on the current mode and retry the link training
> > at a lower link rate/lane count/bpp. This will get called
> > incase the link training fails during the current modeset.
> 
> Based on discussions on #intel-gfx, I would dodge all the problems here
> by having the userspace do the modeset.
> 
> If we add a connector property to indicate the link status (and this
> does not have to be DP or link training specific, really), we can set
> that to "bad", and fire off the hotplug uevent. Then userspace has the
> information to force a modeset on that connector even if the mode has
> not changed. (Credits to Ville for the idea.)
> 
> If we find a solution later on that allows us to handle all of this in
> kernel, we can do so, and remove the property or always report
> "good". Drivers can choose different approaches, depending on the
> capabilities of the hardware, for instance.
> 
> Deferring this to the userspace does not regress anything now, because
> currently we just completely fail and end up with a black screen. The
> property would allow an enlightened userspace to fix that. And userspace
> can't rely on the property being there, as it's currently not there.
> 
> 
> BR,
> Jani.
> 
>

Thanks for your feedback Jani. I will try implementing this, but isn't this going
to require changes in all the userspace drivers (modesetting driver, ChromeOS) to
make use of this new property to trigger another modeset?
How easy would it be to have all the userspace drivers adopt this change?

Regards
Manasi
 
> >
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 58 +++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_atomic_helper.h     |  1 +
> >  2 files changed, 59 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index f936276..0c1614e 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -2895,6 +2895,64 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector,
> >  EXPORT_SYMBOL(drm_atomic_helper_connector_dpms);
> >  
> >  /**
> > + * drm_atomic_helper_connector_modeset - Force a modeset on a connector
> > + * @connector: DRM connector
> > + *
> > + * Provides a way to redo a modeset with the current mode so that it can
> > + * drop the bpp, link rate/lane count and retry the link training.
> > + *
> > + * Returns:
> > + * Returns 0 on success, negative errno numbers on failure.
> > + */
> > +int
> > +drm_atomic_helper_connector_modeset(struct drm_connector *connector)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	struct drm_modeset_acquire_ctx ctx;
> > +	struct drm_atomic_state *state;
> > +	struct drm_connector_state *connector_state;
> > +	struct drm_crtc_state *crtc_state;
> > +	int ret = 0;
> > +
> > +	drm_modeset_acquire_init(&ctx, 0);
> > +	state = drm_atomic_state_alloc(dev);
> > +	if (!state) {
> > +		ret = -ENOMEM;
> > +		goto fail;
> > +	}
> > +	state->acquire_ctx = &ctx;
> > +retry:
> > +	ret = 0;
> > +	connector_state = drm_atomic_get_connector_state(state, connector);
> > +	if (IS_ERR(connector_state)) {
> > +		ret = PTR_ERR(connector_state);
> > +		goto fail;
> > +	}
> > +	if (!connector_state->crtc)
> > +		goto fail;
> > +
> > +	crtc_state = drm_atomic_get_existing_crtc_state(state,
> > +							connector_state->crtc);
> > +	crtc_state->connectors_changed = true;
> > +	ret = drm_atomic_commit(state);
> > +fail:
> > +	if (ret == -EDEADLK) {
> > +		drm_atomic_state_clear(state);
> > +		drm_modeset_backoff(&ctx);
> > +		goto retry;
> > +	}
> > +
> > +	if (state)
> > +		drm_atomic_state_put(state);
> > +
> > +	drm_modeset_drop_locks(&ctx);
> > +	drm_modeset_acquire_fini(&ctx);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_helper_connector_modeset);
> > +
> > +/**
> >   * drm_atomic_helper_best_encoder - Helper for &drm_connector_helper_funcs
> >   *                                  ->best_encoder callback
> >   * @connector: Connector control structure
> > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> > index 7ff92b0..8de24dc 100644
> > --- a/include/drm/drm_atomic_helper.h
> > +++ b/include/drm/drm_atomic_helper.h
> > @@ -126,6 +126,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> >  				uint32_t flags);
> >  int drm_atomic_helper_connector_dpms(struct drm_connector *connector,
> >  				     int mode);
> > +int drm_atomic_helper_connector_modeset(struct drm_connector *connector);
> >  struct drm_encoder *
> >  drm_atomic_helper_best_encoder(struct drm_connector *connector);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx