[libcamera-devel] [RFC PATCH 1/3] libcamera: pipeline: Add device IDs
Jacopo Mondi
jacopo at jmondi.org
Tue Oct 1 10:56:57 CEST 2019
Hi Paul,
On Mon, Sep 30, 2019 at 01:35:18AM -0400, Paul Elder wrote:
> Allow pipeline handlers to declare a list of camera device IDs that they
> support. This list will eventually be used to check against the list of
> camera devices that the system has, but have yet to be initialized by the
> kernel or by udev.
>
> PipelineHandlerFactory exposes a static method to retrieve the list of
> all such camera device IDs from all pipeline handlers.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> src/libcamera/include/pipeline_handler.h | 58 +++++++++++++++++++++++-
> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> src/libcamera/pipeline/uvcvideo.cpp | 2 +-
> src/libcamera/pipeline/vimc.cpp | 2 +-
> src/libcamera/pipeline_handler.cpp | 16 +++++++
> 6 files changed, 79 insertions(+), 5 deletions(-)
>
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 1fdef9ce..f16f37ab 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -30,6 +30,48 @@ class MediaDevice;
> class PipelineHandler;
> class Request;
>
> +class DeviceID
> +{
> +public:
> + DeviceID(std::string type)
> + : type_(type){};
> + ~DeviceID(){};
> +
> + template<class IDType>
> + bool compare(const DeviceID &id) const
> + {
> + return this->type_.compare(id.type_) ||
> + static_cast<const IDType *>(this)->compare(static_cast<const IDType &>(id));
> + }
Why is this different from defining the base class compare() as virtual,
accepting a parameter to the base class in compareDevices() and call the
compare method on the thea first parameter ? If you pass there a PCIDeviceID *
(as a pointer to the DeviceID base class), the PCIDeviceID::compare()
operation, which overrides the base class virtual compare() one, will be
called. Isn't this wat you're trying to achieve here or I'm missing a
piece?
> +
> + const std::string type_;
> +};
> +
> +class PCIDeviceID : public DeviceID
> +{
> +public:
> + PCIDeviceID(uint16_t vendor, uint16_t device)
> + : DeviceID("pci"), vendor_(vendor), device_(device){};
> + ~PCIDeviceID(){};
> +
> + bool compare(const PCIDeviceID &pci) const
> + {
> + uint32_t thisID = (vendor_ << 16) + device_;
> + uint32_t otherID = (pci.vendor_ << 16) + pci.device_;
> +
> + return thisID < otherID;
> + }
> +
> + const uint16_t vendor_;
> + const uint16_t device_;
> +};
> +
> +template<class IDType>
> +bool compareDevices(const DeviceID &id1, const DeviceID &id2)
> +{
> + return id1.compare<IDType>(id2);
> +}
> +
> class CameraData
> {
> public:
> @@ -117,6 +159,10 @@ public:
>
> static void registerType(PipelineHandlerFactory *factory);
> static std::vector<PipelineHandlerFactory *> &factories();
> + static std::vector<std::reference_wrapper<DeviceID>> &getDeviceIDs();
> +
> +protected:
> + virtual std::vector<DeviceID> &deviceIDs() = 0;
>
> private:
> virtual PipelineHandler *createInstance(CameraManager *manager) = 0;
> @@ -124,12 +170,22 @@ private:
> std::string name_;
> };
>
> -#define REGISTER_PIPELINE_HANDLER(handler) \
> +#define P99_PROTECT(...) __VA_ARGS__
I see where it comes from, but P99 refers to C99. If we like to keep
this in the macro, please consider changing its name.
> +
> +#define REGISTER_PIPELINE_HANDLER(handler, cameras) \
> class handler##Factory final : public PipelineHandlerFactory \
> { \
> public: \
> handler##Factory() : PipelineHandlerFactory(#handler) {} \
> \
> +protected: \
> + std::vector<DeviceID> &deviceIDs() \
> + { \
> + static std::vector<DeviceID> handler##Cameras = \
> + std::vector<DeviceID>(cameras); \
> + return handler##Cameras; \
> + } \
> + \
> private: \
> PipelineHandler *createInstance(CameraManager *manager) \
> { \
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 827906d5..562ee9fb 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -1504,6 +1504,8 @@ int CIO2Device::mediaBusToFormat(unsigned int code)
> }
> }
>
> -REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);
> +REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3,
> + P99_PROTECT({ PCIDeviceID(0x8086, 0x1919),
> + PCIDeviceID(0x8086, 0x9D32) }));
What do these IDs refers to ? I haven't found them in the Soraka ACPI
tables (probablt they're not specified there at all).
Are those the IPU3 pipe instances (does the IPU3 firmware exposes two
PCI devices?).
Thanks
j
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index de4ab523..85225f9b 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -513,6 +513,6 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)
> completeRequest(activeCamera_, request);
> }
>
> -REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1);
> +REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1, {});
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 89652105..376eb92a 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -390,6 +390,6 @@ void UVCCameraData::bufferReady(Buffer *buffer)
> pipe_->completeRequest(camera_, request);
> }
>
> -REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC);
> +REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC, {});
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index f26a91f8..75d0236f 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -459,6 +459,6 @@ void VimcCameraData::bufferReady(Buffer *buffer)
> pipe_->completeRequest(camera_, request);
> }
>
> -REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc);
> +REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc, {});
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 3e54aa23..9113bf18 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -593,6 +593,22 @@ std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories()
> return factories;
> }
>
> +std::vector<std::reference_wrapper<DeviceID>> &PipelineHandlerFactory::getDeviceIDs()
> +{
> + static std::vector<std::reference_wrapper<DeviceID>> devices;
> + if (!devices.empty())
> + return devices;
> +
> + std::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();
> +
> + for (PipelineHandlerFactory *factory : factories) {
> + std::vector<DeviceID> &factoryDevices = factory->deviceIDs();
> + devices.insert(devices.end(), factoryDevices.begin(), factoryDevices.end());
> + }
> +
> + return devices;
> +}
> +
> /**
> * \fn PipelineHandlerFactory::createInstance()
> * \brief Create an instance of the PipelineHandler corresponding to the factory
> --
> 2.23.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191001/59842961/attachment.sig>
More information about the libcamera-devel
mailing list