[libcamera-devel] [PATCH 2/3] apps: cam: kms_sink: Drop unique_ptr<> from DRM::AtomicRequest

[Kieran Bingham]

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 at 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 at ideasonboard.com>


Reviewed-by: Kieran Bingham <kieran.bingham at 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