On Tue, Jan 28, 2020 at 12:52:05PM +0100, Noralf Trønnes wrote:
>
>
> Den 28.01.2020 11.45, skrev Daniel Vetter:
> > Instead check for master status, in case we've raced.
> >
> > This is the last exception to the general rule that we restore fbcon
> > only when there's no master active. Compositors are supposed to drop
> > their master status before they switch to a different console back to
> > text mode (or just switch to text mode directly, without a vt switch).
> >
> > This is known to break some subtests of kms_fbcon_fbt in igt, but they're
> > just wrong - it does a graphics/text mode switch for the vt without
> > updating the master status.
> >
> > Also add a comment to the drm_client->restore hook that this is expected
> > going forward from all clients (there's currently just one).
> >
> > v2: Also drop the force in pan_display
> >
> > Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 14 ++------------
> > include/drm/drm_client.h | 5 +++++
> > 2 files changed, 7 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index 4c7cbce7bae7..926187a82255 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -250,17 +250,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> > return 0;
> >
> > mutex_lock(&fb_helper->lock);
> > - /*
> > - * TODO:
> > - * We should bail out here if there is a master by dropping _force.
> > - * Currently these igt tests fail if we do that:
> > - * - kms_fbcon_fbt@psr
> > - * - kms_fbcon_fbt@psr-suspend
> > - *
> > - * So first these tests need to be fixed so they drop master or don't
> > - * have an fd open.
> > - */
> > - ret = drm_client_modeset_commit_force(&fb_helper->client);
> > + ret = drm_client_modeset_commit(&fb_helper->client);
> >
> > do_delayed = fb_helper->delayed_hotplug;
> > if (do_delayed)
> > @@ -1357,7 +1347,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
> >
> > pan_set(fb_helper, var->xoffset, var->yoffset);
> >
> > - ret = drm_client_modeset_commit_force(&fb_helper->client);
> > + ret = drm_client_modeset_commit(&fb_helper->client);
>
> This needs _force because drm_fb_helper_pan_display() already holds the
> locks.
Geez, now I remember again why I did _not_ include this in v1 :-/
> With that fixed:
> Reviewed-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
>
> Maybe a better and more descriptive name would have been
> drm_client_modeset_commit_locked().
This sounds like a good idea, I'll do a patch for this. I'll need to
resend the series anyway to be able to co-test it with the igt side fix.
Thanks for your review.
-Daniel
>
> Noralf.
>
> > if (!ret) {
> > info->var.xoffset = var->xoffset;
> > info->var.yoffset = var->yoffset;
> > diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> > index 5cf2c5dd8b1e..d01d311023ac 100644
> > --- a/include/drm/drm_client.h
> > +++ b/include/drm/drm_client.h
> > @@ -44,6 +44,11 @@ struct drm_client_funcs {
> > * returns zero gets the privilege to restore and no more clients are
> > * called. This callback is not called after @unregister has been called.
> > *
> > + * Note that the core does not guarantee exclusion against concurrent
> > + * drm_open(). Clients need to ensure this themselves, for example by
> > + * using drm_master_internal_acquire() and
> > + * drm_master_internal_release().
> > + *
> > * This callback is optional.
> > */
> > int (*restore)(struct drm_client_dev *client);
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
