| Message ID | 20230423203931.108022-3-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Mon, Apr 24, 2023 at 02:09:30AM +0530, Umang Jain via libcamera-devel wrote: > There is no need to wrap DRM::AtomicRequest in std::unique_ptr<> > in KMSSink::start(). Remove it so that the syntax becomes similar to > what we have in KMSSink::stop(). > > No functional changes intended. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/apps/cam/kms_sink.cpp | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp > index a508977d..2aefec06 100644 > --- a/src/apps/cam/kms_sink.cpp > +++ b/src/apps/cam/kms_sink.cpp > @@ -302,24 +302,22 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format) > > int KMSSink::start() > { > - std::unique_ptr<DRM::AtomicRequest> request; > + DRM::AtomicRequest request(&dev_); While at it, I would move this.. > > int ret = FrameSink::start(); > if (ret < 0) > return ret; > > /* Disable all CRTCs and planes to start from a known valid state. */ > - request = std::make_unique<DRM::AtomicRequest>(&dev_); ... here. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > - > for (const DRM::Crtc &crtc : dev_.crtcs()) > - request->addProperty(&crtc, "ACTIVE", 0); > + request.addProperty(&crtc, "ACTIVE", 0); > > for (const DRM::Plane &plane : dev_.planes()) { > - request->addProperty(&plane, "CRTC_ID", 0); > - request->addProperty(&plane, "FB_ID", 0); > + request.addProperty(&plane, "CRTC_ID", 0); > + request.addProperty(&plane, "FB_ID", 0); > } > > - ret = request->commit(DRM::AtomicRequest::FlagAllowModeset); > + ret = request.commit(DRM::AtomicRequest::FlagAllowModeset); > if (ret < 0) { > std::cerr > << "Failed to disable CRTCs and planes: "
Quoting Laurent Pinchart via libcamera-devel (2023-04-24 01:58:22) > Hi Umang, > > Thank you for the patch. > > On Mon, Apr 24, 2023 at 02:09:30AM +0530, Umang Jain via libcamera-devel wrote: > > There is no need to wrap DRM::AtomicRequest in std::unique_ptr<> > > in KMSSink::start(). Remove it so that the syntax becomes similar to > > what we have in KMSSink::stop(). > > > > No functional changes intended. > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > --- > > src/apps/cam/kms_sink.cpp | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp > > index a508977d..2aefec06 100644 > > --- a/src/apps/cam/kms_sink.cpp > > +++ b/src/apps/cam/kms_sink.cpp > > @@ -302,24 +302,22 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format) > > > > int KMSSink::start() > > { > > - std::unique_ptr<DRM::AtomicRequest> request; > > + DRM::AtomicRequest request(&dev_); > > While at it, I would move this.. > > > > > int ret = FrameSink::start(); > > if (ret < 0) > > return ret; > > > > /* Disable all CRTCs and planes to start from a known valid state. */ > > - request = std::make_unique<DRM::AtomicRequest>(&dev_); > > ... here. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> It looks like this patch could be merged independently too. -- Kieran > > > - > > for (const DRM::Crtc &crtc : dev_.crtcs()) > > - request->addProperty(&crtc, "ACTIVE", 0); > > + request.addProperty(&crtc, "ACTIVE", 0); > > > > for (const DRM::Plane &plane : dev_.planes()) { > > - request->addProperty(&plane, "CRTC_ID", 0); > > - request->addProperty(&plane, "FB_ID", 0); > > + request.addProperty(&plane, "CRTC_ID", 0); > > + request.addProperty(&plane, "FB_ID", 0); > > } > > > > - ret = request->commit(DRM::AtomicRequest::FlagAllowModeset); > > + ret = request.commit(DRM::AtomicRequest::FlagAllowModeset); > > if (ret < 0) { > > std::cerr > > << "Failed to disable CRTCs and planes: " > > -- > Regards, > > Laurent Pinchart
diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp index a508977d..2aefec06 100644 --- a/src/apps/cam/kms_sink.cpp +++ b/src/apps/cam/kms_sink.cpp @@ -302,24 +302,22 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format) int KMSSink::start() { - std::unique_ptr<DRM::AtomicRequest> request; + DRM::AtomicRequest request(&dev_); int ret = FrameSink::start(); if (ret < 0) return ret; /* Disable all CRTCs and planes to start from a known valid state. */ - request = std::make_unique<DRM::AtomicRequest>(&dev_); - for (const DRM::Crtc &crtc : dev_.crtcs()) - request->addProperty(&crtc, "ACTIVE", 0); + request.addProperty(&crtc, "ACTIVE", 0); for (const DRM::Plane &plane : dev_.planes()) { - request->addProperty(&plane, "CRTC_ID", 0); - request->addProperty(&plane, "FB_ID", 0); + request.addProperty(&plane, "CRTC_ID", 0); + request.addProperty(&plane, "FB_ID", 0); } - ret = request->commit(DRM::AtomicRequest::FlagAllowModeset); + ret = request.commit(DRM::AtomicRequest::FlagAllowModeset); if (ret < 0) { std::cerr << "Failed to disable CRTCs and planes: "
There is no need to wrap DRM::AtomicRequest in std::unique_ptr<> in KMSSink::start(). Remove it so that the syntax becomes similar to what we have in KMSSink::stop(). No functional changes intended. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/apps/cam/kms_sink.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)