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

[Rodrigo Vivi]

On Tue, Oct 25, 2016 at 5:09 AM, Jani Nikula
<jani.nikula at linux.intel.com> wrote:
> On Sat, 22 Oct 2016, Manasi Navare <manasi.d.navare at 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.

This is not like going back to UMS times?

> 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.)

This would require changes in all user space drivers out there right?
Ok, maybe faster than fix vblank layer for good...

>
> 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.

So, is this temporary then? While we update the vblank layer?

> 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.

Also this discussion remind me about the PSR where we refused to give control
to userspace or create any property and kmd should control all cases.
Should we revisit that and change userspace to control PSR?

Thanks,
Rodrigo.

>
>
> BR,
> Jani.
>
>
>>
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: Jani Nikula <jani.nikula at linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter at intel.com>
>> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
>> Signed-off-by: Manasi Navare <manasi.d.navare at 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
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br