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

[Eric Curtin]

Hi Laurent,

Thanks for welcoming me.

Another suggested solution would be welcome, let me describe
what happens, it's been reproduced on multiple machines.

So here the the CRTCs output of modetest on my machine:

CRTCs:
id      fb      pos     size
98      256     (0,0)   (1920x1080)
  #0 1920x1080 59.98 1920 1968 2000 2080 1080 1083 1088 1111 138600
flags: nhsync, nvsync; type: preferred, driver
...
167     0       (0,0)   (0x0)
  #0  -nan 0 0 0 0 0 0 0 0 0 flags: ; type:
...
236     0       (0,0)   (0x0)
  #0  -nan 0 0 0 0 0 0 0 0 0 flags: ; type:

So these lines are executed 3 times:

if (plane->supportsFormat(format)) {
    crtc_ = crtc;

crtc_ = 98, break out of loop, crtc_ = 167, break out of loop, crtc_ =
236, break out of loop.

So crtc_ is set in the end to be the one with an id of 236, which doesn't
display anything on my machine but 98 does.

A alternative to fix this would be apprecitaion!

I run like this:

build/src/cam/cam -c1 -DeDP-1 -spixelformat=YUYV -C0

Is mise le meas/Regards,

Eric Curtin



On Wed, 1 Dec 2021 at 17:14, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> 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
>