[v2,1/2] pipeline: rkisp1: Bound RkISP1 path to ISP maximum input
diff mbox series

Message ID 20240913103759.2166-2-umang.jain@ideasonboard.com
State Superseded
Headers show
Series
  • pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline
Related show

Commit Message

Umang Jain Sept. 13, 2024, 10:37 a.m. UTC 
The RkISP1Path class has maximum resolution defined by
RKISP1_RSZ_*_SRC_MAX macros. It might get updated in populateFormats()
by querying the formats on the rkisp1_*path video nodes. However, it
does not take into account the maximum resolution that the ISP can
handle.

For instance on i.MX8MP, 4096x3072 is the maximum supported ISP
resolution however, RkISP1MainPath had the maximum resolution of
RKISP1_RSZ_MP_SRC_MAX, prior to this patch.

Fix it by bounding the maximum resolution of RkISP1Path class by passing
the maximum resolution of the ISP during RkISP1Path::init().

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 15 +++++++++++++--
 src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 13 ++++++++++++-
 src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  2 +-
 3 files changed, 26 insertions(+), 4 deletions(-)

Comments

Laurent Pinchart Sept. 23, 2024, 10:49 p.m. UTC | #1 
Hi Umang,

Thank you for the patch.

On Fri, Sep 13, 2024 at 04:07:58PM +0530, Umang Jain wrote:
> The RkISP1Path class has maximum resolution defined by
> RKISP1_RSZ_*_SRC_MAX macros. It might get updated in populateFormats()
> by querying the formats on the rkisp1_*path video nodes. However, it
> does not take into account the maximum resolution that the ISP can
> handle.
> 
> For instance on i.MX8MP, 4096x3072 is the maximum supported ISP
> resolution however, RkISP1MainPath had the maximum resolution of
> RKISP1_RSZ_MP_SRC_MAX, prior to this patch.
> 
> Fix it by bounding the maximum resolution of RkISP1Path class by passing
> the maximum resolution of the ISP during RkISP1Path::init().
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 15 +++++++++++++--
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 13 ++++++++++++-
>  src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  2 +-
>  3 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 0a794d63..00b0dad8 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1274,6 +1274,17 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  	if (isp_->open() < 0)
>  		return false;
>  
> +	/*
> +	 * Retrieve the ISP maximum input size for config validation in the
> +	 * path classes.
> +	 *
> +	 * The ISP maximum input size is independent of the media bus formats
> +	 * hence, pick the one first entry of ispFormats and its size range.
> +	 */
> +	const V4L2Subdevice::Formats ispFormats = isp_->formats(0);
> +	const SizeRange range = ispFormats.cbegin()->second[0];
> +	const Size ispMaxInputSize = range.max;
> +
>  	/* Locate and open the optional CSI-2 receiver. */
>  	ispSink_ = isp_->entity()->getPadByIndex(0);
>  	if (!ispSink_ || ispSink_->links().empty())
> @@ -1300,10 +1311,10 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  		return false;
>  
>  	/* Locate and open the ISP main and self paths. */
> -	if (!mainPath_.init(media_))
> +	if (!mainPath_.init(media_, ispMaxInputSize))
>  		return false;
>  
> -	if (hasSelfPath_ && !selfPath_.init(media_))
> +	if (hasSelfPath_ && !selfPath_.init(media_, ispMaxInputSize))
>  		return false;
>  
>  	mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index c49017d1..08b39e1e 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -62,7 +62,7 @@ RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
>  {
>  }
>  
> -bool RkISP1Path::init(MediaDevice *media)
> +bool RkISP1Path::init(MediaDevice *media, Size ispMaxInputSize)
>  {
>  	std::string resizer = std::string("rkisp1_resizer_") + name_ + "path";
>  	std::string video = std::string("rkisp1_") + name_ + "path";
> @@ -77,6 +77,17 @@ bool RkISP1Path::init(MediaDevice *media)
>  
>  	populateFormats();
>  
> +	/*
> +	 * The maximum size reported by the video node during populateFormats()
> +	 * is hard coded to a fixed size which can exceed the platform specific

s/platform specific/platform-specific/

> +	 * ISP limitations.
> +	 *
> +	 * The video node should report a maximum size according to the ISP
> +	 * model. This should be fixed in the kernel. For now, restrict the
> +	 * maximum size to the ISP limitations correctly.

Following the discussions on v1, do I understand correctly that the
issue is that the ISP driver correctly reports the maximum size it
supports (rkisp1_isp_enum_frame_size() uses isp->rkisp1->info->max_width
and isp->rkisp1->info->max_height), but that the resizer's output video
node doesn't take that into account ? If so, shouldn't that limitation
be handled in PipelineHandlerRkISP1::generateConfiguration() and/or
RkISP1CameraConfiguration::validate() instead ? The resizer can upscale,
so it seems strange to me to handle this in RkISP1Path. The pipeline
handler already code flow is already quite convoluted, I'd like to make
things clearer.

> +	 */
> +	maxResolution_.boundTo(ispMaxInputSize);
> +
>  	link_ = media->link("rkisp1_isp", 2, resizer, 0);
>  	if (!link_)
>  		return false;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> index 08edefec..13ba4b62 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> @@ -35,7 +35,7 @@ public:
>  	RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
>  		   const Size &minResolution, const Size &maxResolution);
>  
> -	bool init(MediaDevice *media);
> +	bool init(MediaDevice *media, Size ispMaxInputSize);
>  
>  	int setEnabled(bool enable) { return link_->setEnabled(enable); }
>  	bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }
Umang Jain Sept. 24, 2024, 6:10 a.m. UTC | #2 
Hi Laurent,

On 24/09/24 4:19 am, Laurent Pinchart wrote:
> Hi Umang,
>
> Thank you for the patch.
>
> On Fri, Sep 13, 2024 at 04:07:58PM +0530, Umang Jain wrote:
>> The RkISP1Path class has maximum resolution defined by
>> RKISP1_RSZ_*_SRC_MAX macros. It might get updated in populateFormats()
>> by querying the formats on the rkisp1_*path video nodes. However, it
>> does not take into account the maximum resolution that the ISP can
>> handle.
>>
>> For instance on i.MX8MP, 4096x3072 is the maximum supported ISP
>> resolution however, RkISP1MainPath had the maximum resolution of
>> RKISP1_RSZ_MP_SRC_MAX, prior to this patch.
>>
>> Fix it by bounding the maximum resolution of RkISP1Path class by passing
>> the maximum resolution of the ISP during RkISP1Path::init().
>>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>   src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 15 +++++++++++++--
>>   src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 13 ++++++++++++-
>>   src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  2 +-
>>   3 files changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> index 0a794d63..00b0dad8 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> @@ -1274,6 +1274,17 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>>   	if (isp_->open() < 0)
>>   		return false;
>>   
>> +	/*
>> +	 * Retrieve the ISP maximum input size for config validation in the
>> +	 * path classes.
>> +	 *
>> +	 * The ISP maximum input size is independent of the media bus formats
>> +	 * hence, pick the one first entry of ispFormats and its size range.
>> +	 */
>> +	const V4L2Subdevice::Formats ispFormats = isp_->formats(0);
>> +	const SizeRange range = ispFormats.cbegin()->second[0];
>> +	const Size ispMaxInputSize = range.max;
>> +
>>   	/* Locate and open the optional CSI-2 receiver. */
>>   	ispSink_ = isp_->entity()->getPadByIndex(0);
>>   	if (!ispSink_ || ispSink_->links().empty())
>> @@ -1300,10 +1311,10 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>>   		return false;
>>   
>>   	/* Locate and open the ISP main and self paths. */
>> -	if (!mainPath_.init(media_))
>> +	if (!mainPath_.init(media_, ispMaxInputSize))
>>   		return false;
>>   
>> -	if (hasSelfPath_ && !selfPath_.init(media_))
>> +	if (hasSelfPath_ && !selfPath_.init(media_, ispMaxInputSize))
>>   		return false;
>>   
>>   	mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
>> index c49017d1..08b39e1e 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
>> @@ -62,7 +62,7 @@ RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
>>   {
>>   }
>>   
>> -bool RkISP1Path::init(MediaDevice *media)
>> +bool RkISP1Path::init(MediaDevice *media, Size ispMaxInputSize)
>>   {
>>   	std::string resizer = std::string("rkisp1_resizer_") + name_ + "path";
>>   	std::string video = std::string("rkisp1_") + name_ + "path";
>> @@ -77,6 +77,17 @@ bool RkISP1Path::init(MediaDevice *media)
>>   
>>   	populateFormats();
>>   
>> +	/*
>> +	 * The maximum size reported by the video node during populateFormats()
>> +	 * is hard coded to a fixed size which can exceed the platform specific
> s/platform specific/platform-specific/
>
>> +	 * ISP limitations.
>> +	 *
>> +	 * The video node should report a maximum size according to the ISP
>> +	 * model. This should be fixed in the kernel. For now, restrict the
>> +	 * maximum size to the ISP limitations correctly.
> Following the discussions on v1, do I understand correctly that the
> issue is that the ISP driver correctly reports the maximum size it
> supports (rkisp1_isp_enum_frame_size() uses isp->rkisp1->info->max_width
> and isp->rkisp1->info->max_height), but that the resizer's output video
> node doesn't take that into account ? If so, shouldn't that limitation

yes, that's the crux of the issue here. Hence, this seems more like a 
kernel fix?
> be handled in PipelineHandlerRkISP1::generateConfiguration() and/or
> RkISP1CameraConfiguration::validate() instead ? The resizer can upscale,
> so it seems strange to me to handle this in RkISP1Path. The pipeline
> handler already code flow is already quite convoluted, I'd like to make
> things clearer.

The resizer can upscale yes, but should it be upscaling beyond ISP (or 
even maximum sensor resolution) limits ? No, right ?

>
>> +	 */
>> +	maxResolution_.boundTo(ispMaxInputSize);
>> +
>>   	link_ = media->link("rkisp1_isp", 2, resizer, 0);
>>   	if (!link_)
>>   		return false;
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
>> index 08edefec..13ba4b62 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
>> @@ -35,7 +35,7 @@ public:
>>   	RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
>>   		   const Size &minResolution, const Size &maxResolution);
>>   
>> -	bool init(MediaDevice *media);
>> +	bool init(MediaDevice *media, Size ispMaxInputSize);
>>   
>>   	int setEnabled(bool enable) { return link_->setEnabled(enable); }
>>   	bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }
Laurent Pinchart Sept. 25, 2024, 11:55 a.m. UTC | #3 
Hi Umang,

On Tue, Sep 24, 2024 at 11:40:09AM +0530, Umang Jain wrote:
> On 24/09/24 4:19 am, Laurent Pinchart wrote:
> > On Fri, Sep 13, 2024 at 04:07:58PM +0530, Umang Jain wrote:
> >> The RkISP1Path class has maximum resolution defined by
> >> RKISP1_RSZ_*_SRC_MAX macros. It might get updated in populateFormats()
> >> by querying the formats on the rkisp1_*path video nodes. However, it
> >> does not take into account the maximum resolution that the ISP can
> >> handle.
> >>
> >> For instance on i.MX8MP, 4096x3072 is the maximum supported ISP
> >> resolution however, RkISP1MainPath had the maximum resolution of
> >> RKISP1_RSZ_MP_SRC_MAX, prior to this patch.
> >>
> >> Fix it by bounding the maximum resolution of RkISP1Path class by passing
> >> the maximum resolution of the ISP during RkISP1Path::init().
> >>
> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >> ---
> >>   src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 15 +++++++++++++--
> >>   src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 13 ++++++++++++-
> >>   src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  2 +-
> >>   3 files changed, 26 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> >> index 0a794d63..00b0dad8 100644
> >> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> >> @@ -1274,6 +1274,17 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> >>   	if (isp_->open() < 0)
> >>   		return false;
> >>   
> >> +	/*
> >> +	 * Retrieve the ISP maximum input size for config validation in the
> >> +	 * path classes.
> >> +	 *
> >> +	 * The ISP maximum input size is independent of the media bus formats
> >> +	 * hence, pick the one first entry of ispFormats and its size range.
> >> +	 */
> >> +	const V4L2Subdevice::Formats ispFormats = isp_->formats(0);
> >> +	const SizeRange range = ispFormats.cbegin()->second[0];
> >> +	const Size ispMaxInputSize = range.max;
> >> +
> >>   	/* Locate and open the optional CSI-2 receiver. */
> >>   	ispSink_ = isp_->entity()->getPadByIndex(0);
> >>   	if (!ispSink_ || ispSink_->links().empty())
> >> @@ -1300,10 +1311,10 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> >>   		return false;
> >>   
> >>   	/* Locate and open the ISP main and self paths. */
> >> -	if (!mainPath_.init(media_))
> >> +	if (!mainPath_.init(media_, ispMaxInputSize))
> >>   		return false;
> >>   
> >> -	if (hasSelfPath_ && !selfPath_.init(media_))
> >> +	if (hasSelfPath_ && !selfPath_.init(media_, ispMaxInputSize))
> >>   		return false;
> >>   
> >>   	mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> >> index c49017d1..08b39e1e 100644
> >> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> >> @@ -62,7 +62,7 @@ RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
> >>   {
> >>   }
> >>   
> >> -bool RkISP1Path::init(MediaDevice *media)
> >> +bool RkISP1Path::init(MediaDevice *media, Size ispMaxInputSize)
> >>   {
> >>   	std::string resizer = std::string("rkisp1_resizer_") + name_ + "path";
> >>   	std::string video = std::string("rkisp1_") + name_ + "path";
> >> @@ -77,6 +77,17 @@ bool RkISP1Path::init(MediaDevice *media)
> >>   
> >>   	populateFormats();
> >>   
> >> +	/*
> >> +	 * The maximum size reported by the video node during populateFormats()
> >> +	 * is hard coded to a fixed size which can exceed the platform specific
> >
> > s/platform specific/platform-specific/
> >
> >> +	 * ISP limitations.
> >> +	 *
> >> +	 * The video node should report a maximum size according to the ISP
> >> +	 * model. This should be fixed in the kernel. For now, restrict the
> >> +	 * maximum size to the ISP limitations correctly.
> >
> > Following the discussions on v1, do I understand correctly that the
> > issue is that the ISP driver correctly reports the maximum size it
> > supports (rkisp1_isp_enum_frame_size() uses isp->rkisp1->info->max_width
> > and isp->rkisp1->info->max_height), but that the resizer's output video
> > node doesn't take that into account ? If so, shouldn't that limitation
> 
> yes, that's the crux of the issue here. Hence, this seems more like a 
> kernel fix?
>
> > be handled in PipelineHandlerRkISP1::generateConfiguration() and/or
> > RkISP1CameraConfiguration::validate() instead ? The resizer can upscale,
> > so it seems strange to me to handle this in RkISP1Path. The pipeline
> > handler already code flow is already quite convoluted, I'd like to make
> > things clearer.
> 
> The resizer can upscale yes, but should it be upscaling beyond ISP (or 
> even maximum sensor resolution) limits ? No, right ?

On the kernel side, I don't think we should limit upscaling to the
sensor resolution or maximum ISP input size. The kernel driver should
expose the full capabilities of the hardware. If the resizer can upscale
to more than 4k x 3k, we shouldn't restrict it.

What we can (and should) do in the kernel is to restrict upscaling to
the maximum supported by the hardware, in case upscaling too much would
make the ISP behave incorrectly. I suspect that that limit would be
dynamic though, not a static value we could access here, but instead
something to be handled at configuration validation time.

The problem can be segmented, you don't have to handle everything in one
go. We could decide to start without upscaling support in libcamera for
instance. As long as the code is kept clean, and limitations are
properly documented, I'm fine with that.

> >> +	 */
> >> +	maxResolution_.boundTo(ispMaxInputSize);
> >> +
> >>   	link_ = media->link("rkisp1_isp", 2, resizer, 0);
> >>   	if (!link_)
> >>   		return false;
> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> >> index 08edefec..13ba4b62 100644
> >> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> >> @@ -35,7 +35,7 @@ public:
> >>   	RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
> >>   		   const Size &minResolution, const Size &maxResolution);
> >>   
> >> -	bool init(MediaDevice *media);
> >> +	bool init(MediaDevice *media, Size ispMaxInputSize);
> >>   
> >>   	int setEnabled(bool enable) { return link_->setEnabled(enable); }
> >>   	bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 0a794d63..00b0dad8 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -1274,6 +1274,17 @@  bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 	if (isp_->open() < 0)
 		return false;
 
+	/*
+	 * Retrieve the ISP maximum input size for config validation in the
+	 * path classes.
+	 *
+	 * The ISP maximum input size is independent of the media bus formats
+	 * hence, pick the one first entry of ispFormats and its size range.
+	 */
+	const V4L2Subdevice::Formats ispFormats = isp_->formats(0);
+	const SizeRange range = ispFormats.cbegin()->second[0];
+	const Size ispMaxInputSize = range.max;
+
 	/* Locate and open the optional CSI-2 receiver. */
 	ispSink_ = isp_->entity()->getPadByIndex(0);
 	if (!ispSink_ || ispSink_->links().empty())
@@ -1300,10 +1311,10 @@  bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 		return false;
 
 	/* Locate and open the ISP main and self paths. */
-	if (!mainPath_.init(media_))
+	if (!mainPath_.init(media_, ispMaxInputSize))
 		return false;
 
-	if (hasSelfPath_ && !selfPath_.init(media_))
+	if (hasSelfPath_ && !selfPath_.init(media_, ispMaxInputSize))
 		return false;
 
 	mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
index c49017d1..08b39e1e 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
@@ -62,7 +62,7 @@  RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
 {
 }
 
-bool RkISP1Path::init(MediaDevice *media)
+bool RkISP1Path::init(MediaDevice *media, Size ispMaxInputSize)
 {
 	std::string resizer = std::string("rkisp1_resizer_") + name_ + "path";
 	std::string video = std::string("rkisp1_") + name_ + "path";
@@ -77,6 +77,17 @@  bool RkISP1Path::init(MediaDevice *media)
 
 	populateFormats();
 
+	/*
+	 * The maximum size reported by the video node during populateFormats()
+	 * is hard coded to a fixed size which can exceed the platform specific
+	 * ISP limitations.
+	 *
+	 * The video node should report a maximum size according to the ISP
+	 * model. This should be fixed in the kernel. For now, restrict the
+	 * maximum size to the ISP limitations correctly.
+	 */
+	maxResolution_.boundTo(ispMaxInputSize);
+
 	link_ = media->link("rkisp1_isp", 2, resizer, 0);
 	if (!link_)
 		return false;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
index 08edefec..13ba4b62 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
@@ -35,7 +35,7 @@  public:
 	RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
 		   const Size &minResolution, const Size &maxResolution);
 
-	bool init(MediaDevice *media);
+	bool init(MediaDevice *media, Size ispMaxInputSize);
 
 	int setEnabled(bool enable) { return link_->setEnabled(enable); }
 	bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }