dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
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

  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).