Brian Starkey <brian.starkey@xxxxxxx> writes:
> Hi Boris,
>
> Sorry lost track of this thread.
>
>
> On Tue, Jun 06, 2017 at 09:13:00PM +0200, Boris Brezillon wrote:
>>Hi Brian,
>>
>>Le Mon, 5 Jun 2017 12:25:50 +0100,
>>Brian Starkey <brian.starkey@xxxxxxx> a écrit :
>>
>>> Hi Boris,
>>>
>>> I can't speak for the HW-specific details, but the writeback part
>>> looks pretty good (and familiar ;-) to me. There certainly seems to be
>>> some scope for code-sharing there, but I think it's fine not to do it
>>> yet.
>>
>>Agreed.
>>
>>>
>>> I've a further query below about the handling of CRTC events.
>>>
>>
>>[...]
>>
>>> >+
>>> >+void vc4_txp_atomic_commit(struct drm_device *dev,
>>> >+ struct drm_atomic_state *old_state)
>>> >+{
>>> >+ struct vc4_dev *vc4 = to_vc4_dev(dev);
>>> >+ struct vc4_txp *txp = vc4->txp;
>>> >+ struct drm_connector_state *conn_state = txp->connector.base.state;
>>> >+ struct drm_display_mode *mode;
>>> >+ struct drm_gem_cma_object *gem;
>>> >+ struct drm_framebuffer *fb;
>>> >+ u32 ctrl = TXP_GO | TXP_EI;
>>> >+
>>> >+ /* Writeback connector is disabled, nothing to do. */
>>> >+ if (!conn_state->crtc)
>>> >+ return;
>>> >+
>>> >+ /* Writeback connector is enabled, but has no FB assigned to it. Fake a
>>> >+ * vblank event to complete ->flip_done.
>>> >+ */
>>> >+ if (!conn_state->writeback_job || !conn_state->writeback_job->fb) {
>>> >+ vc4_crtc_eof_event(conn_state->crtc);
>>>
>>> I'm not sure about hiding away the one-shot thing like this. If the
>>> CRTC remains "active" from the API point of view, I'd expect it to be
>>> able to keep generating VBLANK events.
>>>
>>> I don't know how to do if, but if there were some notion of
>>> "auto-disabling" CRTCs then this quirk would go away, and you'd also
>>> be able to enforce that the CRTC can't be enabled without a writeback
>>> framebuffer.
>>
>>I don't have a strong opinion on "auto-disabling CRTC" vs "fake VBLANK
>>events". Note that I'm already faking a VBLANK event when activating
>>writeback, because there's no such concept at the HVS/TXP level. We
>>do have EOF (End Of Frame) and writeback done events, but these are
>>quite independent from the VBLANK events generated by the pixelvalve
>>block (the block responsible for generating the HSYNC/VSYNC signals).
>>
>>>
>>> I'm also not sure how (if?) this works today with a CRTC driving a DSI
>>> command-mode panel. Does the CRTC keep generating VBLANKs even when
>>> there are no updates?
>>
>>I don't know. Is this mandatory to have VBLANK events? I mean, the
>>core is using VBLANK events to detect when page flips have been done,
>>that is, when old framebuffers are unused and new ones started to be
>>fetched by the CRTC, but on some HW, VBLANK is not the only way to
>>detect such situations. The question is, are there other situations
>>where VBLANK events are required, or is there an implicit rule stating
>>that a CRTC has to generate VBLANK events at a vrefresh rate even when
>>the display is actually not updated when nothing changes?
>
> I'm not sure how widely relied upon it is, but I'd expect that there's
> a requirement to make sure DRM_IOCTL_WAIT_VBLANK works. I _think_ that
> means the CRTC should generate events at vrefresh rate if there's a
> wait request outstanding that depends on that.
In our case, there's no vrefresh rate, though. Just completion.
I've been trying to come up with a usecase for vblank events on
writeack, and the best I have is: to enable VNC-style capture of a
complete hardware rendering stack, we could run modesetting against the
writeback connector and do one-shot writebacks when damage comes in.
You would want GL apps to be throttled to the frame capture rate, so X
needs to implement waits for at least a swapinterval of 1 (I don't see
how greater than 1 would make any sense)
However, given that you'd be triggering writebacks on damage, and using
the fence to trigger the next wait for damage and writeback already, I
think you'd use that set of code for Present/DRI2 swapinterval support,
not the current vblank path.
My current inclination would be to throw -EINVAL for userspace vblank
requests on writeback conncetor pipes.
Attachment:
signature.asc
Description: PGP signature
