Message ID | 20220808214851.30804-1-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Quoting Laurent Pinchart (2022-08-08 22:48:51) > The KMS sink currently displays the frame buffer on the top-left corner > of the screen, resulting in either a black area on the bottom and right > sides (if the frame buffer is smaller than the display resolution) of in > a restricted field of view (if the frame buffer is larger than the > display resolution). Improve this by scaling the frame buffer to full > screen if supported, and aligning the crop rectangle to the frame buffer > center if the field of view needs to be restricted. > > The implementation test fiyr possible composition options, from best to ... tests for possible ... ? > worst. The tests are performed when the camera is started, as testing > atomic commits requires access to frame buffer objects, which are not > available at configure time. Changing this would require either a large > refactoring of the cam application to provide frame buffers earlier, or > extending the KMS API to support testing commits with dummy buffer > objects. Both are candidates for later development. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Eric Curtin <ecurtin@redhat.com> > --- > Changes since v1: > > - Maintain the aspect ratio if possible > --- > src/cam/kms_sink.cpp | 109 +++++++++++++++++++++++++++++++++++++++---- > src/cam/kms_sink.h | 8 ++++ > 2 files changed, 109 insertions(+), 8 deletions(-) > > diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp > index 16435ede6b6a..17e2fa69bff7 100644 > --- a/src/cam/kms_sink.cpp > +++ b/src/cam/kms_sink.cpp > @@ -284,6 +284,94 @@ int KMSSink::stop() > return FrameSink::stop(); > } > > +bool KMSSink::testModeSet(DRM::FrameBuffer *drmBuffer, > + const libcamera::Rectangle &src, > + const libcamera::Rectangle &dst) > +{ > + DRM::AtomicRequest drmRequest{ &dev_ }; > + > + drmRequest.addProperty(connector_, "CRTC_ID", crtc_->id()); > + > + drmRequest.addProperty(crtc_, "ACTIVE", 1); > + drmRequest.addProperty(crtc_, "MODE_ID", mode_->toBlob(&dev_)); > + > + drmRequest.addProperty(plane_, "CRTC_ID", crtc_->id()); > + drmRequest.addProperty(plane_, "FB_ID", drmBuffer->id()); > + drmRequest.addProperty(plane_, "SRC_X", src.x << 16); > + drmRequest.addProperty(plane_, "SRC_Y", src.y << 16); > + drmRequest.addProperty(plane_, "SRC_W", src.width << 16); > + drmRequest.addProperty(plane_, "SRC_H", src.height << 16); > + drmRequest.addProperty(plane_, "CRTC_X", dst.x); > + drmRequest.addProperty(plane_, "CRTC_Y", dst.y); > + drmRequest.addProperty(plane_, "CRTC_W", dst.width); > + drmRequest.addProperty(plane_, "CRTC_H", dst.height); > + > + return !drmRequest.commit(DRM::AtomicRequest::FlagAllowModeset | > + DRM::AtomicRequest::FlagTestOnly); > +} > + > +bool KMSSink::setupComposition(DRM::FrameBuffer *drmBuffer) > +{ > + /* > + * Test composition options, from most to least desirable, to select the > + * best one. > + */ > + const libcamera::Rectangle framebuffer{ size_ }; > + const libcamera::Rectangle display{ 0, 0, mode_->hdisplay, mode_->vdisplay }; > + > + /* 1. Scale the frame buffer to full screen, preserving aspect ratio. */ > + libcamera::Rectangle src = framebuffer; > + libcamera::Rectangle dst = display.size().boundedToAspectRatio(framebuffer.size()) > + .centeredTo(display.center()); > + That sounds much better to me! Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + if (testModeSet(drmBuffer, src, dst)) { > + std::cout << "KMS: full-screen scaled output, square pixels" > + << std::endl; > + src_ = src; > + dst_ = dst; > + return true; > + } > + > + /* > + * 2. Scale the frame buffer to full screen, without preserving aspect > + * ratio. > + */ > + src = framebuffer; > + dst = display; > + > + if (testModeSet(drmBuffer, src, dst)) { > + std::cout << "KMS: full-screen scaled output, non-square pixels" > + << std::endl; > + src_ = src; > + dst_ = dst; > + return true; > + } > + > + /* 3. Center the frame buffer on the display. */ > + src = display.size().centeredTo(framebuffer.center()).boundedTo(framebuffer); > + dst = framebuffer.size().centeredTo(display.center()).boundedTo(display); > + > + if (testModeSet(drmBuffer, src, dst)) { > + std::cout << "KMS: centered output" << std::endl; > + src_ = src; > + dst_ = dst; > + return true; > + } > + > + /* 4. Align the frame buffer on the top-left of the display. */ > + src = framebuffer.boundedTo(display); > + dst = display.boundedTo(framebuffer); > + > + if (testModeSet(drmBuffer, src, dst)) { > + std::cout << "KMS: top-left aligned output" << std::endl; > + src_ = src; > + dst_ = dst; > + return true; > + } > + > + return false; > +} > + > bool KMSSink::processRequest(libcamera::Request *camRequest) > { > /* > @@ -307,20 +395,25 @@ bool KMSSink::processRequest(libcamera::Request *camRequest) > > if (!active_ && !queued_) { > /* Enable the display pipeline on the first frame. */ > + if (!setupComposition(drmBuffer)) { > + std::cerr << "Failed to setup composition" << std::endl; > + return true; > + } > + > drmRequest->addProperty(connector_, "CRTC_ID", crtc_->id()); > > drmRequest->addProperty(crtc_, "ACTIVE", 1); > drmRequest->addProperty(crtc_, "MODE_ID", mode_->toBlob(&dev_)); > > drmRequest->addProperty(plane_, "CRTC_ID", crtc_->id()); > - drmRequest->addProperty(plane_, "SRC_X", 0 << 16); > - drmRequest->addProperty(plane_, "SRC_Y", 0 << 16); > - drmRequest->addProperty(plane_, "SRC_W", size_.width << 16); > - drmRequest->addProperty(plane_, "SRC_H", size_.height << 16); > - drmRequest->addProperty(plane_, "CRTC_X", 0); > - drmRequest->addProperty(plane_, "CRTC_Y", 0); > - drmRequest->addProperty(plane_, "CRTC_W", size_.width); > - drmRequest->addProperty(plane_, "CRTC_H", size_.height); > + drmRequest->addProperty(plane_, "SRC_X", src_.x << 16); > + drmRequest->addProperty(plane_, "SRC_Y", src_.y << 16); > + drmRequest->addProperty(plane_, "SRC_W", src_.width << 16); > + drmRequest->addProperty(plane_, "SRC_H", src_.height << 16); > + drmRequest->addProperty(plane_, "CRTC_X", dst_.x); > + drmRequest->addProperty(plane_, "CRTC_Y", dst_.y); > + drmRequest->addProperty(plane_, "CRTC_W", dst_.width); > + drmRequest->addProperty(plane_, "CRTC_H", dst_.height); > > flags |= DRM::AtomicRequest::FlagAllowModeset; > } > diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h > index 8f5f08667cea..76c4e611bf85 100644 > --- a/src/cam/kms_sink.h > +++ b/src/cam/kms_sink.h > @@ -50,6 +50,11 @@ private: > > int selectPipeline(const libcamera::PixelFormat &format); > int configurePipeline(const libcamera::PixelFormat &format); > + bool testModeSet(DRM::FrameBuffer *drmBuffer, > + const libcamera::Rectangle &src, > + const libcamera::Rectangle &dst); > + bool setupComposition(DRM::FrameBuffer *drmBuffer); > + > void requestComplete(DRM::AtomicRequest *request); > > DRM::Device dev_; > @@ -63,6 +68,9 @@ private: > libcamera::Size size_; > unsigned int stride_; > > + libcamera::Rectangle src_; > + libcamera::Rectangle dst_; > + > std::map<libcamera::FrameBuffer *, std::unique_ptr<DRM::FrameBuffer>> buffers_; > > std::mutex lock_; > -- > Regards, > > Laurent Pinchart >
diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp index 16435ede6b6a..17e2fa69bff7 100644 --- a/src/cam/kms_sink.cpp +++ b/src/cam/kms_sink.cpp @@ -284,6 +284,94 @@ int KMSSink::stop() return FrameSink::stop(); } +bool KMSSink::testModeSet(DRM::FrameBuffer *drmBuffer, + const libcamera::Rectangle &src, + const libcamera::Rectangle &dst) +{ + DRM::AtomicRequest drmRequest{ &dev_ }; + + drmRequest.addProperty(connector_, "CRTC_ID", crtc_->id()); + + drmRequest.addProperty(crtc_, "ACTIVE", 1); + drmRequest.addProperty(crtc_, "MODE_ID", mode_->toBlob(&dev_)); + + drmRequest.addProperty(plane_, "CRTC_ID", crtc_->id()); + drmRequest.addProperty(plane_, "FB_ID", drmBuffer->id()); + drmRequest.addProperty(plane_, "SRC_X", src.x << 16); + drmRequest.addProperty(plane_, "SRC_Y", src.y << 16); + drmRequest.addProperty(plane_, "SRC_W", src.width << 16); + drmRequest.addProperty(plane_, "SRC_H", src.height << 16); + drmRequest.addProperty(plane_, "CRTC_X", dst.x); + drmRequest.addProperty(plane_, "CRTC_Y", dst.y); + drmRequest.addProperty(plane_, "CRTC_W", dst.width); + drmRequest.addProperty(plane_, "CRTC_H", dst.height); + + return !drmRequest.commit(DRM::AtomicRequest::FlagAllowModeset | + DRM::AtomicRequest::FlagTestOnly); +} + +bool KMSSink::setupComposition(DRM::FrameBuffer *drmBuffer) +{ + /* + * Test composition options, from most to least desirable, to select the + * best one. + */ + const libcamera::Rectangle framebuffer{ size_ }; + const libcamera::Rectangle display{ 0, 0, mode_->hdisplay, mode_->vdisplay }; + + /* 1. Scale the frame buffer to full screen, preserving aspect ratio. */ + libcamera::Rectangle src = framebuffer; + libcamera::Rectangle dst = display.size().boundedToAspectRatio(framebuffer.size()) + .centeredTo(display.center()); + + if (testModeSet(drmBuffer, src, dst)) { + std::cout << "KMS: full-screen scaled output, square pixels" + << std::endl; + src_ = src; + dst_ = dst; + return true; + } + + /* + * 2. Scale the frame buffer to full screen, without preserving aspect + * ratio. + */ + src = framebuffer; + dst = display; + + if (testModeSet(drmBuffer, src, dst)) { + std::cout << "KMS: full-screen scaled output, non-square pixels" + << std::endl; + src_ = src; + dst_ = dst; + return true; + } + + /* 3. Center the frame buffer on the display. */ + src = display.size().centeredTo(framebuffer.center()).boundedTo(framebuffer); + dst = framebuffer.size().centeredTo(display.center()).boundedTo(display); + + if (testModeSet(drmBuffer, src, dst)) { + std::cout << "KMS: centered output" << std::endl; + src_ = src; + dst_ = dst; + return true; + } + + /* 4. Align the frame buffer on the top-left of the display. */ + src = framebuffer.boundedTo(display); + dst = display.boundedTo(framebuffer); + + if (testModeSet(drmBuffer, src, dst)) { + std::cout << "KMS: top-left aligned output" << std::endl; + src_ = src; + dst_ = dst; + return true; + } + + return false; +} + bool KMSSink::processRequest(libcamera::Request *camRequest) { /* @@ -307,20 +395,25 @@ bool KMSSink::processRequest(libcamera::Request *camRequest) if (!active_ && !queued_) { /* Enable the display pipeline on the first frame. */ + if (!setupComposition(drmBuffer)) { + std::cerr << "Failed to setup composition" << std::endl; + return true; + } + drmRequest->addProperty(connector_, "CRTC_ID", crtc_->id()); drmRequest->addProperty(crtc_, "ACTIVE", 1); drmRequest->addProperty(crtc_, "MODE_ID", mode_->toBlob(&dev_)); drmRequest->addProperty(plane_, "CRTC_ID", crtc_->id()); - drmRequest->addProperty(plane_, "SRC_X", 0 << 16); - drmRequest->addProperty(plane_, "SRC_Y", 0 << 16); - drmRequest->addProperty(plane_, "SRC_W", size_.width << 16); - drmRequest->addProperty(plane_, "SRC_H", size_.height << 16); - drmRequest->addProperty(plane_, "CRTC_X", 0); - drmRequest->addProperty(plane_, "CRTC_Y", 0); - drmRequest->addProperty(plane_, "CRTC_W", size_.width); - drmRequest->addProperty(plane_, "CRTC_H", size_.height); + drmRequest->addProperty(plane_, "SRC_X", src_.x << 16); + drmRequest->addProperty(plane_, "SRC_Y", src_.y << 16); + drmRequest->addProperty(plane_, "SRC_W", src_.width << 16); + drmRequest->addProperty(plane_, "SRC_H", src_.height << 16); + drmRequest->addProperty(plane_, "CRTC_X", dst_.x); + drmRequest->addProperty(plane_, "CRTC_Y", dst_.y); + drmRequest->addProperty(plane_, "CRTC_W", dst_.width); + drmRequest->addProperty(plane_, "CRTC_H", dst_.height); flags |= DRM::AtomicRequest::FlagAllowModeset; } diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h index 8f5f08667cea..76c4e611bf85 100644 --- a/src/cam/kms_sink.h +++ b/src/cam/kms_sink.h @@ -50,6 +50,11 @@ private: int selectPipeline(const libcamera::PixelFormat &format); int configurePipeline(const libcamera::PixelFormat &format); + bool testModeSet(DRM::FrameBuffer *drmBuffer, + const libcamera::Rectangle &src, + const libcamera::Rectangle &dst); + bool setupComposition(DRM::FrameBuffer *drmBuffer); + void requestComplete(DRM::AtomicRequest *request); DRM::Device dev_; @@ -63,6 +68,9 @@ private: libcamera::Size size_; unsigned int stride_; + libcamera::Rectangle src_; + libcamera::Rectangle dst_; + std::map<libcamera::FrameBuffer *, std::unique_ptr<DRM::FrameBuffer>> buffers_; std::mutex lock_;