From: Daniel Vetter <daniel@ffwll.ch>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm: Check CRTC compatibility in setplane
Date: Wed, 23 Apr 2014 20:33:40 +0200 [thread overview]
Message-ID: <20140423183340.GH10722@phenom.ffwll.local> (raw)
In-Reply-To: <20140423182604.GD19628@intel.com>
On Wed, Apr 23, 2014 at 11:26:04AM -0700, Matt Roper wrote:
> On Wed, Apr 23, 2014 at 08:03:50PM +0200, Daniel Vetter wrote:
> > On Wed, Apr 23, 2014 at 10:03:59AM -0700, Matt Roper wrote:
> > > The DRM core setplane code should check that the plane is usable on the
> > > specified CRTC before calling into the driver.
> > >
> > > Prior to this patch, a plane's possible_crtcs field was purely
> > > informational for userspace and was never actually verified at the
> > > kernel level (aside from the primary plane helper).
> > >
> > > Cc: dri-devel@lists.freedesktop.org
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> >
> > Do you have a nasty igt somewhere which tries to use a plane on the wrong
> > crtc? Especially useful since our i915 code and hw relies on this.
> > -Daniel
>
> Not yet; I'll add/modify a test to include that. I'm still working on
> some other i-g-t test updates for the primary plane stuff based on your
> previous feedback.
>
> Speaking of i-g-t testing, what is the expected behavior if someone
> issues a pageflip ioctl while the primary plane is disabled? I'd expect
> it to return an error, but the -EBUSY currently returned by the DRM core
> seems a bit confusing/misleading in my opinion. The comments indicate
> that the existing assumption is that crtc->primary->fb == NULL indicates
> a hotplug event that userspace hasn't noticed yet, although now we
> clearly have other ways to hit that error, so I'm not sure EBUSY is
> really the right response.
That comment is outdated since nowadays the kernel doesn't randomly kill a
crtc if its outputs gets unplugged. I think a simple -EINVAL should be
good. We'd need to update kms_flip with that new value though.
A quick grep through the intel ddx shows that we don't really depend on
this either way. -EBUSY for a disabled primary plane (whether the crtc is
on or not doesn't matter imo) really makes no sense.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
prev parent reply other threads:[~2014-04-23 18:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-23 17:03 [PATCH 1/3] drm: Check CRTC compatibility in setplane Matt Roper
2014-04-23 17:04 ` [PATCH 2/3] drm/plane-helper: Add drm_primary_helper_check_update() Matt Roper
2014-04-23 18:39 ` Ville Syrjälä
2014-04-23 17:04 ` [PATCH 3/3] drm/i915: Intel-specific primary plane handling (v5) Matt Roper
2014-04-23 18:03 ` [Intel-gfx] [PATCH 1/3] drm: Check CRTC compatibility in setplane Daniel Vetter
2014-04-23 18:26 ` Matt Roper
2014-04-23 18:33 ` Daniel Vetter [this message]
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=20140423183340.GH10722@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=matthew.d.roper@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox