All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: jilaiw@codeaurora.org
Cc: linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 2/3] drm:msm: Initial Add Writeback Support
Date: Wed, 8 Apr 2015 10:07:07 +0200	[thread overview]
Message-ID: <20150408080707.GN6354@phenom.ffwll.local> (raw)
In-Reply-To: <93d3802305fa348738dae7f458c3fb56.squirrel@www.codeaurora.org>

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.

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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: jilaiw@codeaurora.org
Cc: Rob Clark <robdclark@gmail.com>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 2/3] drm:msm: Initial Add Writeback Support
Date: Wed, 8 Apr 2015 10:07:07 +0200	[thread overview]
Message-ID: <20150408080707.GN6354@phenom.ffwll.local> (raw)
In-Reply-To: <93d3802305fa348738dae7f458c3fb56.squirrel@www.codeaurora.org>

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.

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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

  parent reply	other threads:[~2015-04-08  8:07 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-01 21:12 [PATCH 2/3] drm:msm: Initial Add Writeback Support Jilai Wang
2015-04-01 21:12 ` Jilai Wang
2015-04-02  9:40 ` Paul Bolle
2015-04-02 17:58   ` jilaiw
2015-04-02 17:58     ` jilaiw
2015-04-02 18:24     ` Paul Bolle
2015-04-02 18:24       ` Paul Bolle
2015-04-02 18:42       ` Rob Clark
2015-04-02 18:42         ` Rob Clark
2015-04-02 18:54         ` jilaiw
2015-04-02 18:54           ` jilaiw
2015-04-02 18:54         ` jilaiw
2015-04-02 18:54           ` jilaiw
2015-04-02 11:48 ` Emil Velikov
2015-04-02 11:48   ` Emil Velikov
2015-04-02 18:07   ` jilaiw
2015-04-02 18:07     ` jilaiw
2015-04-02 18:19     ` Rob Clark
2015-04-02 18:19       ` Rob Clark
2015-04-02 22:29     ` Emil Velikov
2015-04-02 22:29       ` Emil Velikov
2015-04-02 14:29 ` Rob Clark
2015-04-02 14:29   ` Rob Clark
2015-04-07  5:59   ` Daniel Vetter
2015-04-07 15:55     ` jilaiw
2015-04-07 15:55       ` jilaiw
2015-04-07 18:09       ` [PATCH 2/3] drm:msm: Initial Add Writeback Support (V2) Jilai Wang
2015-08-19 19:00         ` Rob Clark
2015-08-25  7:05           ` Daniel Vetter
2015-08-25  7:05             ` Daniel Vetter
2015-08-25 19:11             ` Rob Clark
2015-08-26 12:06               ` Daniel Vetter
2015-08-26 12:06                 ` Daniel Vetter
2015-04-08  8:07       ` Daniel Vetter [this message]
2015-04-08  8:07         ` [PATCH 2/3] drm:msm: Initial Add Writeback Support Daniel Vetter
2015-04-08 14:40         ` jilaiw
2015-04-08 14:40           ` jilaiw

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=20150408080707.GN6354@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jilaiw@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.