Re: [PATCH v2] drm/atomic: Fix bookkeeping with TEST_ONLY, v2.

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Stone <daniel@fooishbar.org>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v2] drm/atomic: Fix bookkeeping with TEST_ONLY, v2.
Date: Thu, 27 Aug 2015 17:34:44 +0300	[thread overview]
Message-ID: <20150827143444.GH5176@intel.com> (raw)
In-Reply-To: <CAPj87rMyi0yLRt17nCmbiM=11PVitNiVjdOybGtm+KBLZ6UYDA@mail.gmail.com>

On Thu, Aug 27, 2015 at 03:28:53PM +0100, Daniel Stone wrote:
> Hi,
> 
> On 27 August 2015 at 15:09, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Aug 27, 2015 at 04:00:05PM +0200, Maarten Lankhorst wrote:
> >> Op 27-08-15 om 15:50 schreef Ville Syrjälä:
> >> > I don't think so. Speaking for i915, I think we've just rejected legacy page
> >> > flips entirely with the pipe is off on account of drm_vblank_get() failing.
> >> No atomic driver handles this case correctly. You can't get vblank events with the crtc off.
> >
> > I don't understand what you're saying. Should there be a comma after
> > "No" ? If not, then I'm not sure what they don't handle.
> 
> I think you're arguing over the definition of 'correctly' perhaps?
> 
> >> >> I don't see why this should be relaxed. It just complicates things and you have nothing to stick in for the vblank counter.
> >> > We could stick the last vbl count/timestamp in there.
> >> >
> >> > Not allowing means userspace is forced to consider the dpms state
> >> > whenever it wants to call the atomic ioctl.
> >> Userspace was the one turning off the crtc in the first place; it shouldn't continue flipping but preserve power. :-)
> >
> > Meh. Much simpler to write code when you don't have to worry about such
> > details.
> >
> > In the kernel it should amount to
> > if (!pipe_active)
> >         send_event
> 
> No, thankyou. Asking for an event, having the request succeed, and
> never getting an event, is a deathtrap.

Obviously the event gets sent if the operation succeeds. I never
suggested anything else.

> PAGE_FLIP_EVENT should mean
> that either an event gets delivered for every CRTC in crtc_state, or
> the request getting rejected. Nothing else.
> 
> > We anyway need something like that for the crtc getting disabled case,
> > don't we?
> 
> Yes, which I was getting at previously. We know when the CRTC gets
> disabled - we have to, in order to sensibly unpin etc - so that's when
> the event gets sent.
> 
> Cheers,
> Daniel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx