All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Stone <daniel@fooishbar.org>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [Intel-gfx] [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY"
Date: Thu, 11 Feb 2021 16:14:02 +0000	[thread overview]
Message-ID: <CAPj87rOZgAD0FJOD+4X2vniV8PP6dMLSn6fDzaf-rP00JYoDeA@mail.gmail.com> (raw)
In-Reply-To: <YCP2l7PDMTE2a0Eh@intel.com>

On Wed, 10 Feb 2021 at 15:07, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Feb 10, 2021 at 01:38:45PM +0000, Simon Ser wrote:
> > The WARN_ON only happens if allow_modeset == false. If allow_modeset == true,
> > then the driver is allowed to steal an unrelated pipe.
> >
> > Maybe i915 is stealing a pipe without allow_modeset?
>
> No. All page flips etc. will have to get split up internally
> between multiple crtcs.

I think this is the salient point.

> So I think there's basically three options:
> a) massive rewrite of i915 to bypass even more of drm_atomic stuff
> b) allow i915 to silence that warning, which opens up the question
>    whether the warn is doing any good if it can just be bypassed
> c) nuke the warning entirely
>
> a) is not going to happen, and it would any way allow i915 to
> do things any which way it wants without tripping the warn,
> rendering the warn entirely toothless.
>
> Hmm. Maybe there is a d) which would be to ignore all crtcs
> that are not logically enabled in the warn? Not sure if that
> could allow something to slit through that people want it to
> catch?

So from what I understand, if I enable CRTC 44 and the driver
magically decides to split it up as a 'big-joiner' output, it will
also steal CRTC 50 to work as the other half of the output. Then if I
attach plane 47 to CRTC 44, posting a FB to plane 47 will result in me
getting atomic completion events for both CRTC 44 and CRTC 50?

That's not OK from a userspace perspective. If you want to do magic to
create the illusion of a single CRTC, then that magic needs to be
consistent. At the moment it's the worst kind of magic: it does
implicit things under the hood for you, but then leaks all of the
behind-the-scenes detail into userspace.

Continuing with that would force us all to just ignore whatever events
we see, because we can't reason about what they may be or why they're
generated. Which doesn't allow for any kind of best practice in
userspace.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Stone <daniel@fooishbar.org>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Simon Ser <contact@emersion.fr>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [Intel-gfx] [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY"
Date: Thu, 11 Feb 2021 16:14:02 +0000	[thread overview]
Message-ID: <CAPj87rOZgAD0FJOD+4X2vniV8PP6dMLSn6fDzaf-rP00JYoDeA@mail.gmail.com> (raw)
In-Reply-To: <YCP2l7PDMTE2a0Eh@intel.com>

On Wed, 10 Feb 2021 at 15:07, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Feb 10, 2021 at 01:38:45PM +0000, Simon Ser wrote:
> > The WARN_ON only happens if allow_modeset == false. If allow_modeset == true,
> > then the driver is allowed to steal an unrelated pipe.
> >
> > Maybe i915 is stealing a pipe without allow_modeset?
>
> No. All page flips etc. will have to get split up internally
> between multiple crtcs.

I think this is the salient point.

> So I think there's basically three options:
> a) massive rewrite of i915 to bypass even more of drm_atomic stuff
> b) allow i915 to silence that warning, which opens up the question
>    whether the warn is doing any good if it can just be bypassed
> c) nuke the warning entirely
>
> a) is not going to happen, and it would any way allow i915 to
> do things any which way it wants without tripping the warn,
> rendering the warn entirely toothless.
>
> Hmm. Maybe there is a d) which would be to ignore all crtcs
> that are not logically enabled in the warn? Not sure if that
> could allow something to slit through that people want it to
> catch?

So from what I understand, if I enable CRTC 44 and the driver
magically decides to split it up as a 'big-joiner' output, it will
also steal CRTC 50 to work as the other half of the output. Then if I
attach plane 47 to CRTC 44, posting a FB to plane 47 will result in me
getting atomic completion events for both CRTC 44 and CRTC 50?

That's not OK from a userspace perspective. If you want to do magic to
create the illusion of a single CRTC, then that magic needs to be
consistent. At the moment it's the worst kind of magic: it does
implicit things under the hood for you, but then leaks all of the
behind-the-scenes detail into userspace.

Continuing with that would force us all to just ignore whatever events
we see, because we can't reason about what they may be or why they're
generated. Which doesn't allow for any kind of best practice in
userspace.

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2021-02-11 16:14 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-10  0:14 [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY" Manasi Navare
2021-02-10  0:14 ` [Intel-gfx] " Manasi Navare
2021-02-10  2:05 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2021-02-10  6:19 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2021-02-10 13:16 ` [Intel-gfx] [PATCH] " Daniel Vetter
2021-02-10 13:16   ` Daniel Vetter
2021-02-10 13:38   ` Simon Ser
2021-02-10 13:38     ` Simon Ser
2021-02-10 15:07     ` Ville Syrjälä
2021-02-10 15:07       ` Ville Syrjälä
2021-02-10 23:26       ` Navare, Manasi
2021-02-10 23:26         ` Navare, Manasi
2021-02-11 15:46         ` Daniel Vetter
2021-02-11 15:46           ` Daniel Vetter
2021-02-11 16:14       ` Daniel Stone [this message]
2021-02-11 16:14         ` Daniel Stone
2021-02-11 16:55         ` Ville Syrjälä
2021-02-11 16:55           ` Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPj87rOZgAD0FJOD+4X2vniV8PP6dMLSn6fDzaf-rP00JYoDeA@mail.gmail.com \
    --to=daniel@fooishbar.org \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.