| Message ID | 20210906225636.14683-24-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On 06/09/2021 23:56, Laurent Pinchart wrote: > Now that libcamera supports per-plane offsets, pass the values to > drmModeAddFB2(). Any further implications from this? Will there be a specific effect? or does it just mean that DRM can allocate buffers for more of our formats? Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/cam/drm.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp > index ac47b8bd3287..d5a75d039fd8 100644 > --- a/src/cam/drm.cpp > +++ b/src/cam/drm.cpp > @@ -623,7 +623,7 @@ std::unique_ptr<FrameBuffer> Device::createFrameBuffer( > fb->planes_.push_back({ handle }); > > handles[i] = handle; > - offsets[i] = 0; /* TODO */ > + offsets[i] = plane.offset; > ++i; > } > >
Hi Kieran, On Tue, Sep 07, 2021 at 12:43:43PM +0100, Kieran Bingham wrote: > On 06/09/2021 23:56, Laurent Pinchart wrote: > > Now that libcamera supports per-plane offsets, pass the values to > > drmModeAddFB2(). > > Any further implications from this? > Will there be a specific effect? or does it just mean that DRM can > allocate buffers for more of our formats? There's no particular implication other than the fact that the KMS sink in cam is now fixed for multi-planar formats :-) Without the offset it wouldn't render NV12 correctly when planes are stored in the same dmabuf. We still allocate buffers using the FrameBufferAllocator even when using the KMS sink, so this isn't related to allocation. > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > --- > > src/cam/drm.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp > > index ac47b8bd3287..d5a75d039fd8 100644 > > --- a/src/cam/drm.cpp > > +++ b/src/cam/drm.cpp > > @@ -623,7 +623,7 @@ std::unique_ptr<FrameBuffer> Device::createFrameBuffer( > > fb->planes_.push_back({ handle }); > > > > handles[i] = handle; > > - offsets[i] = 0; /* TODO */ > > + offsets[i] = plane.offset; > > ++i; > > } > >
On 07/09/2021 15:47, Laurent Pinchart wrote: > Hi Kieran, > > On Tue, Sep 07, 2021 at 12:43:43PM +0100, Kieran Bingham wrote: >> On 06/09/2021 23:56, Laurent Pinchart wrote: >>> Now that libcamera supports per-plane offsets, pass the values to >>> drmModeAddFB2(). >> >> Any further implications from this? >> Will there be a specific effect? or does it just mean that DRM can >> allocate buffers for more of our formats? > > There's no particular implication other than the fact that the KMS sink > in cam is now fixed for multi-planar formats :-) Without the offset it > wouldn't render NV12 correctly when planes are stored in the same > dmabuf. Perhaps you could add that to the commit message; "The KMS sink in cam is now capable of rendering multi-planar formats." > We still allocate buffers using the FrameBufferAllocator even when using > the KMS sink, so this isn't related to allocation. > >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >>> --- >>> src/cam/drm.cpp | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp >>> index ac47b8bd3287..d5a75d039fd8 100644 >>> --- a/src/cam/drm.cpp >>> +++ b/src/cam/drm.cpp >>> @@ -623,7 +623,7 @@ std::unique_ptr<FrameBuffer> Device::createFrameBuffer( >>> fb->planes_.push_back({ handle }); >>> >>> handles[i] = handle; >>> - offsets[i] = 0; /* TODO */ >>> + offsets[i] = plane.offset; >>> ++i; >>> } >>> >
Hi Kieran, On Tue, Sep 07, 2021 at 04:45:10PM +0100, Kieran Bingham wrote: > On 07/09/2021 15:47, Laurent Pinchart wrote: > > On Tue, Sep 07, 2021 at 12:43:43PM +0100, Kieran Bingham wrote: > >> On 06/09/2021 23:56, Laurent Pinchart wrote: > >>> Now that libcamera supports per-plane offsets, pass the values to > >>> drmModeAddFB2(). > >> > >> Any further implications from this? > >> Will there be a specific effect? or does it just mean that DRM can > >> allocate buffers for more of our formats? > > > > There's no particular implication other than the fact that the KMS sink > > in cam is now fixed for multi-planar formats :-) Without the offset it > > wouldn't render NV12 correctly when planes are stored in the same > > dmabuf. > > Perhaps you could add that to the commit message; > > "The KMS sink in cam is now capable of rendering multi-planar formats." Done. > > We still allocate buffers using the FrameBufferAllocator even when using > > the KMS sink, so this isn't related to allocation. > > > >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> > >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > >>> --- > >>> src/cam/drm.cpp | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp > >>> index ac47b8bd3287..d5a75d039fd8 100644 > >>> --- a/src/cam/drm.cpp > >>> +++ b/src/cam/drm.cpp > >>> @@ -623,7 +623,7 @@ std::unique_ptr<FrameBuffer> Device::createFrameBuffer( > >>> fb->planes_.push_back({ handle }); > >>> > >>> handles[i] = handle; > >>> - offsets[i] = 0; /* TODO */ > >>> + offsets[i] = plane.offset; > >>> ++i; > >>> } > >>>
diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp index ac47b8bd3287..d5a75d039fd8 100644 --- a/src/cam/drm.cpp +++ b/src/cam/drm.cpp @@ -623,7 +623,7 @@ std::unique_ptr<FrameBuffer> Device::createFrameBuffer( fb->planes_.push_back({ handle }); handles[i] = handle; - offsets[i] = 0; /* TODO */ + offsets[i] = plane.offset; ++i; }