[libcamera-devel] [PATCH v4] cam: kms_sink: Remove limitation that camera and display must match

[Laurent Pinchart]

Hi Eric,

Thank you for the patch.

On Fri, Jun 17, 2022 at 10:41:27AM +0100, Eric Curtin via libcamera-devel wrote:
> There is a limitation that requires input and output to be pixel
> for pixel identical in terms of height and width. Remove this
> limitation to enable more hardware that doesn't match. Just start
> drawing from top left 0, 0 corner.
> 
> Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> ---
> Changes in v4:
> - Change commit message to say top left
> - Spaces to tabs
> 
> Changes in v3:
> - Much simplified version of the patch where we just attempt to
>   draw from point 0, 0. Only in the case where we do not find a
>   matching mode. Can expand to do centralization, scaling, etc.
>   in further patches if needs be.
> 
> Changes in v2:
> - Tested and support drawing from negative pixel range
>   kernel parameter (video=960x540 at 60) was useful here
> ---
>  src/cam/kms_sink.cpp | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp
> index 7add81a6..985b5f2e 100644
> --- a/src/cam/kms_sink.cpp
> +++ b/src/cam/kms_sink.cpp
> @@ -119,15 +119,20 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)
>  						      mode.vdisplay == cfg.size.height;
>  				       });
>  	if (iter == modes.end()) {
> -		std::cerr << "No mode matching " << cfg.size << std::endl;
> -		return -EINVAL;
> +		if (modes.empty()) {
> +			std::cerr << "No modes\n";
> +			return -EINVAL;
> +		}
> +
> +		mode_ = &modes[0];
> +	} else {
> +		mode_ = &*iter;
>  	}
>  
>  	int ret = configurePipeline(cfg.pixelFormat);
>  	if (ret < 0)
>  		return ret;
>  
> -	mode_ = &*iter;
>  	size_ = cfg.size;
>  	stride_ = cfg.stride;

I've tested this on my system, selecting a stream size that doesn't
match any of the modes supported by the display. It works, but selects a
mode that is very far from the stream size. I've experimented a bit with
code that picks the mode closest to the stream size. I'll post a v5 with
that code for your review.

>  
> @@ -295,12 +300,12 @@ bool KMSSink::processRequest(libcamera::Request *camRequest)
>  		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", mode_->hdisplay << 16);
> -		drmRequest->addProperty(plane_, "SRC_H", mode_->vdisplay << 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", mode_->hdisplay);
> -		drmRequest->addProperty(plane_, "CRTC_H", mode_->vdisplay);
> +		drmRequest->addProperty(plane_, "CRTC_W", size_.width);
> +		drmRequest->addProperty(plane_, "CRTC_H", size_.height);
>  
>  		flags |= DRM::AtomicRequest::FlagAllowModeset;
>  	}

-- 
Regards,

Laurent Pinchart