From: Brian Starkey <brian.starkey@arm.com>
To: Eric Anholt <eric@anholt.net>
Cc: liviu.dudau@arm.com, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org, hverkuil@xs4all.nl,
linux-media@vger.kernel.org
Subject: Re: [RFC PATCH 00/11] Introduce writeback connectors
Date: Wed, 12 Oct 2016 08:30:37 +0100 [thread overview]
Message-ID: <20161012072643.GA17390@localhost> (raw)
In-Reply-To: <87d1j6emmd.fsf@eliezer.anholt.net>
Hi Eric,
On Tue, Oct 11, 2016 at 12:01:14PM -0700, Eric Anholt wrote:
>Brian Starkey <brian.starkey@arm.com> writes:
>
>> Hi,
>>
>> This RFC series introduces a new connector type:
>> DRM_MODE_CONNECTOR_WRITEBACK
>> It is a follow-on from a previous discussion: [1]
>>
>> Writeback connectors are used to expose the memory writeback engines
>> found in some display controllers, which can write a CRTC's
>> composition result to a memory buffer.
>> This is useful e.g. for testing, screen-recording, screenshots,
>> wireless display, display cloning, memory-to-memory composition.
>>
>> Patches 1-7 include the core framework changes required, and patches
>> 8-11 implement a writeback connector for the Mali-DP writeback engine.
>> The Mali-DP patches depend on this other series: [2].
>>
>> The connector is given the FB_ID property for the output framebuffer,
>> and two new read-only properties: PIXEL_FORMATS and
>> PIXEL_FORMATS_SIZE, which expose the supported framebuffer pixel
>> formats of the engine.
>>
>> The EDID property is not exposed for writeback connectors.
>>
>> Writeback connector usage:
>> --------------------------
>> Due to connector routing changes being treated as "full modeset"
>> operations, any client which wishes to use a writeback connector
>> should include the connector in every modeset. The writeback will not
>> actually become active until a framebuffer is attached.
>>
>> The writeback itself is enabled by attaching a framebuffer to the
>> FB_ID property of the connector. The driver must then ensure that the
>> CRTC content of that atomic commit is written into the framebuffer.
>>
>> The writeback works in a one-shot mode with each atomic commit. This
>> prevents the same content from being written multiple times.
>> In some cases (front-buffer rendering) there might be a desire for
>> continuous operation - I think a property could be added later for
>> this kind of control.
>>
>> Writeback can be disabled by setting FB_ID to zero.
>
>I think this sounds great, and the interface is just right IMO.
>
Thanks, glad you like it! Hopefully you're equally agreeable with the
changes Daniel has been suggesting.
>I don't really see a use for continuous mode -- a sequence of one-shots
>makes a lot more sense because then you can know what data has changed,
>which anyone trying to use the writeback buffer would need to know.
>
Agreed - we've never found a use for it.
>> Known issues:
>> -------------
>> * I'm not sure what "DPMS" should mean for writeback connectors.
>> It could be used to disable writeback (even when a framebuffer is
>> attached), or it could be hidden entirely (which would break the
>> legacy DPMS call for writeback connectors).
>> * With Daniel's recent re-iteration of the userspace API rules, I
>> fully expect to provide some userspace code to support this. The
>> question is what, and where? We want to use writeback for testing,
>> so perhaps some tests in igt is suitable.
>> * Documentation. Probably some portion of this cover letter needs to
>> make it into Documentation/
>> * Synchronisation. Our hardware will finish the writeback by the next
>> vsync. I've not implemented fence support here, but it would be an
>> obvious addition.
>
>My hardware won't necessarily finish by the next vsync -- it trickles
>out at whatever rate it can find memory bandwidth to get the job done,
>and fires an interrupt when it's finished.
>
Is it bounded? You presumably have to finish the write-out before you
can change any input buffers?
>So I would like some definition for how syncing works. One answer would
>be that these flips don't trigger their pageflip events until the
>writeback is done (so I need to collect both the vsync irq and the
>writeback irq before sending). Another would be that manage an
>independent fence for the writeback fb, so that you still immediately
>know when framebuffers from the previous scanout-only frame are idle.
>
I much prefer the sound of the explicit fence approach.
Hopefully we can agree that a new atomic commit can't be completed
whilst there's a writeback ongoing, otherwise managing the fence and
framebuffer lifetime sounds really tricky - they'd need to be decoupled
from the atomic_state and outlive the commit that spawned them.
Cheers,
-Brian
>Also, tests for this in igt, please. Writeback in igt will give us so
>much more ability to cover KMS functionality on non-Intel hardware.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2016-10-12 7:31 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-11 14:53 [RFC PATCH 00/11] Introduce writeback connectors Brian Starkey
2016-10-11 14:53 ` [RFC PATCH 01/11] drm: Add writeback connector type Brian Starkey
2016-10-11 14:53 ` [RFC PATCH 02/11] drm/fb-helper: Skip writeback connectors Brian Starkey
2016-10-11 15:44 ` Daniel Vetter
2016-10-11 16:47 ` Brian Starkey
2016-10-11 16:56 ` Daniel Vetter
2016-10-11 14:54 ` [RFC PATCH 03/11] drm: Extract CRTC/plane disable from drm_framebuffer_remove Brian Starkey
2016-10-11 14:54 ` [RFC PATCH 04/11] drm: Add __drm_framebuffer_remove_atomic Brian Starkey
2016-10-11 15:51 ` Daniel Vetter
2016-10-11 14:54 ` [RFC PATCH 05/11] drm: Add fb to connector state Brian Starkey
2016-10-11 14:54 ` [RFC PATCH 06/11] drm: Expose fb_id property for writeback connectors Brian Starkey
2016-10-11 14:54 ` [RFC PATCH 07/11] drm: Add writeback-connector pixel format properties Brian Starkey
2016-10-11 15:48 ` Daniel Vetter
2016-10-11 14:54 ` [RFC PATCH 08/11] drm: mali-dp: Rename malidp_input_format Brian Starkey
2016-10-11 14:54 ` [RFC PATCH 09/11] drm: mali-dp: Add RGB writeback formats for DP550/DP650 Brian Starkey
2016-10-11 14:54 ` [RFC PATCH 10/11] drm: mali-dp: Add support for writeback on DP550/DP650 Brian Starkey
2016-10-11 14:54 ` [RFC PATCH 11/11] drm: mali-dp: Add writeback connector Brian Starkey
2016-10-11 15:43 ` [RFC PATCH 00/11] Introduce writeback connectors Daniel Vetter
2016-10-11 16:43 ` Brian Starkey
2016-10-11 17:01 ` Daniel Vetter
2016-10-11 19:44 ` Brian Starkey
2016-10-11 20:02 ` Daniel Vetter
2016-10-11 21:24 ` Brian Starkey
2016-10-12 6:56 ` Daniel Vetter
2016-10-13 9:47 ` [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset Brian Starkey
2016-10-13 14:54 ` Alex Deucher
2016-10-17 6:07 ` Daniel Vetter
2016-10-11 16:25 ` [RFC PATCH 00/11] Introduce writeback connectors Ville Syrjälä
2016-10-11 16:29 ` Daniel Vetter
2016-10-11 19:01 ` Eric Anholt
2016-10-12 7:30 ` Brian Starkey [this message]
2016-10-13 17:32 ` Eric Anholt
2016-10-14 10:50 ` Archit Taneja
2016-10-14 12:39 ` Brian Starkey
2016-10-14 12:50 ` Ville Syrjälä
2016-10-14 14:56 ` Daniel Vetter
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=20161012072643.GA17390@localhost \
--to=brian.starkey@arm.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=eric@anholt.net \
--cc=hverkuil@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=liviu.dudau@arm.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;
as well as URLs for NNTP newsgroup(s).