Re: [PATCH 2/3] drm:msm: Initial Add Writeback Support

jilaiw
2015-04-08T14:40:25Z

> On Tue, Apr 07, 2015 at 03:55:45PM -0000, jilaiw@codeaurora.org wrote:
>> > On Thu, Apr 02, 2015 at 10:29:52AM -0400, Rob Clark wrote:
>> >> So, from a quick look, it seems like there is a lot of potential to
>> >> split the v4l part out into some drm helpers.. it looks pretty
>> >> generic(ish), or at least it could be with some strategically placed
>> >> vfuncs in drm_v4l2_helper_funcs.
>> >>
>> >> I do think we need to figure out the auth/security situation. We
>> >> probably don't want to let arbitrary processes open a v4l device and
>> >> snoop on the screen contents. We perhaps could re-use the dri2 drm
>> >> auth stuff (v4l2_drm_get_magic ioctl?). Or, well, it would be nice
>> if
>> >> the wb device could be made to not exist in /dev at all, and
>> >> pre-open'd fd returned from an ioctl on the drm device, but not
>> really
>> >> sure if that is possible (or too weird). Once the compositor process
>> >> has the v4l device open and authenticated somehow, I expect it would
>> >> use fd passing to pass the fd off to a trusted helper process.
>> >
>> > Please don't resurrect the magic stuff ;-)
>> >
>> > Anyway I discussed this a bit with Laurent and we figured the best way
>> to
>> > wire up writeback support is by using drm framebuffers. Then you can
>> use
>> > atomic flips to create a new snapshot. Of course that won't work with
>> hw
>> > where writeback is continuous, there v4l is a much better fit. And we
>> also
>> > have hardware where some v4l pipeline could directly feed into a drm
>> > output pipeline, so we need a generic way to connect v4l and drm
>> anyway.
>> > For that I think we should add a new flag to addfb2 (or a new
>> addfbv4l)
>> > which creates a magic framebuffer from a v4l input/output. Some values
>> > like stride don't make sense in such a virtual framebuffer, but pixel
>> > format and size are all needed.
>> >
>> > This way we don't need parallel abis for single-shot writeback
>> directly
>> > into framebuffers and for continuous writeback through v4l, we can
>> reuse
>> > the same drm framebuffer ones. And this also solves the security
>> issues
>> > since no one can start writeback without the drm device owner's
>> consent,
>> > so no need to reinvent anything there. And with atomic we already have
>> > almost everything there: For the writeback framebuffer we only need a
>> new
>> > "WRITEBACK" property (which takes an fb id) and the small extension to
>> > create v4l-backed framebuffers.
>> >
>> > Cheers, Daniel
>> > --
>> > Daniel Vetter
>> > Software Engineer, Intel Corporation
>> > http://blog.ffwll.ch
>> >
>> Hi Daniel,
>>
>> 1. This change is to implement a continuous writeback.
>> 2. As you said, we need "a generic way to connect v4l and drm".
>> Especially how to share the buffer information between v4l and drm for
>> writeback output.
>>
>> Below are just some details of this change:
>>
>> In current implementation, I expect the output buffer is dma buffer
>> which could be from GEM object (drm) or from video encoder (V4l). Once
>> the buffer is queued into V4l driver, it will be converted into a GEM
>> object and then pass it to drm as writeback output buffer. Once the
>> buffer is captured, it will notify V4l driver to let user dequeue
>> buffer.
>>
>> Drm will notice there is a Virtual Connector (maybe a new type WRITEBACK
>> can be added), but it will only be "connected" until V4l
>> starts streaming.
>
> Yes we definitely should add a new connector type WRITEBACK. And just the
> connector kinda works for your hw design where writeback works like a
> separate encoder. But there's also hw out there where any crtc can be
> written back, and for those cases we need explicit properties. Then
> there's also the one-shot vs. continuous issues.
yes, this change is specific for msm chip. In order to cover the other
writeback use cases, new properties and framework changes are required.

>
> Given all that I still think you want an explicit drm framebuffer to
> connect the kms and the v4l side of things. That would also help a bit
> with making it clear which v4l connects to which drm device.
yes, it will help.

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>