[libcamera-devel] [PATCH] cam: kms_sink: select valid crtc

[Laurent Pinchart]

Hi Eric,

Thank you for the patch, and welcome to libcamera.

On Wed, Dec 01, 2021 at 04:44:13PM +0000, Eric Curtin wrote:
> When looping through possibleCrtcs on my machine, the only valid crtc is
> actually the first one. This can be identified by checking if the clock
> variable is non-zero.
> 
> Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> ---
>  src/cam/drm.cpp      | 2 +-
>  src/cam/drm.h        | 2 ++
>  src/cam/kms_sink.cpp | 4 ++++
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp
> index f2530091..808e5a87 100644
> --- a/src/cam/drm.cpp
> +++ b/src/cam/drm.cpp
> @@ -128,7 +128,7 @@ std::unique_ptr<Blob> Mode::toBlob(Device *dev) const
>  }
>  
>  Crtc::Crtc(Device *dev, const drmModeCrtc *crtc, unsigned int index)
> -	: Object(dev, crtc->crtc_id, Object::TypeCrtc), index_(index)
> +	: Object(dev, crtc->crtc_id, Object::TypeCrtc), index_(index), crtc_(crtc)
>  {
>  }
>  
> diff --git a/src/cam/drm.h b/src/cam/drm.h
> index de57e445..d05e6d13 100644
> --- a/src/cam/drm.h
> +++ b/src/cam/drm.h
> @@ -149,12 +149,14 @@ public:
>  	Crtc(Device *dev, const drmModeCrtc *crtc, unsigned int index);
>  
>  	unsigned int index() const { return index_; }
> +	uint32_t clock() const { return crtc_->mode.clock; }
>  	const std::vector<const Plane *> &planes() const { return planes_; }
>  
>  private:
>  	friend Device;
>  
>  	unsigned int index_;
> +	const drmModeCrtc *crtc_;
>  	std::vector<const Plane *> planes_;
>  };
>  
> diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp
> index d30fba78..aa0b7a3c 100644
> --- a/src/cam/kms_sink.cpp
> +++ b/src/cam/kms_sink.cpp
> @@ -166,6 +166,10 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format)
>  	 */
>  	for (const DRM::Encoder *encoder : connector_->encoders()) {
>  		for (const DRM::Crtc *crtc : encoder->possibleCrtcs()) {
> +			if (!crtc->clock()) {
> +				continue;
> +			}

I don't think this is quite right. When the Crtc instance is created,
drmModeCrtc.mode contains the currently configured mode. Any CRTC that
is not enabled when the cam application is started will thus be skipped,
even if it can be used.

On a side note, no need for curly braces.

> +
>  			for (const DRM::Plane *plane : crtc->planes()) {
>  				if (plane->type() != DRM::Plane::TypePrimary)
>  					continue;

-- 
Regards,

Laurent Pinchart