| Message ID | 20240902-imx290-avail-v3-2-b32a12799fed@skidata.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: i2c: imx290: check for availability in probe() | expand |
Hi Benjamin On Mon, 2 Sept 2024 at 16:58, <bbara93@gmail.com> wrote: > > From: Benjamin Bara <benjamin.bara@skidata.com> > > For now, the driver activates the first mode (1080p) as current active > mode in probe(). This e.g. means that one cannot set VBLANK below 45 > (vmax_min - height), although theoretically the minimum is 30 (720p > mode). Define the absolute possible/supported ranges to have them > available later. Currently the driver will set the ranges for VBLANK and HBLANK whenever the mode changes. How is it helpful to fake these numbers? Seeing as they aren't reflecting anything useful, they may as well all be 0. > Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> > --- > Changes since v2: > - new > --- > drivers/media/i2c/imx290.c | 36 ++++++++++++++++++++++++++++++++---- > 1 file changed, 32 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > index 1c97f9650eb4..466492bab600 100644 > --- a/drivers/media/i2c/imx290.c > +++ b/drivers/media/i2c/imx290.c > @@ -499,6 +499,10 @@ static const struct imx290_clk_cfg imx290_720p_clock_config[] = { > }; > > /* Mode configs */ > +#define WIDTH_720P 1280 > +#define HEIGHT_720P 720 > +#define MINIMUM_WIDTH WIDTH_720P > +#define MINIMUM_HEIGHT HEIGHT_720P > static const struct imx290_mode imx290_modes_2lanes[] = { > { > .width = 1920, > @@ -512,8 +516,8 @@ static const struct imx290_mode imx290_modes_2lanes[] = { > .clk_cfg = imx290_1080p_clock_config, > }, > { > - .width = 1280, > - .height = 720, > + .width = WIDTH_720P, > + .height = HEIGHT_720P, > .hmax_min = 3300, > .vmax_min = 750, > .link_freq_index = FREQ_INDEX_720P, > @@ -537,8 +541,8 @@ static const struct imx290_mode imx290_modes_4lanes[] = { > .clk_cfg = imx290_1080p_clock_config, > }, > { > - .width = 1280, > - .height = 720, > + .width = WIDTH_720P, > + .height = HEIGHT_720P, > .hmax_min = 3300, > .vmax_min = 750, > .link_freq_index = FREQ_INDEX_720P, > @@ -846,6 +850,30 @@ static const char * const imx290_test_pattern_menu[] = { > "000/555h Toggle Pattern", > }; > > +/* absolute supported control ranges */ > +#define HBLANK_MAX (IMX290_HMAX_MAX - MINIMUM_WIDTH) > +#define VBLANK_MAX (IMX290_VMAX_MAX - MINIMUM_HEIGHT) > +static unsigned int imx290_get_blank_min(const struct imx290 *imx290, bool v) > +{ This function is never used in this patch. I'm surprised the compiler doesn't throw an error on a static function not being used. You first use it in patch 4 "Introduce initial "off" mode & link freq" > + const struct imx290_mode *modes = imx290_modes_ptr(imx290); > + unsigned int min = UINT_MAX; > + int i; > + > + for (i = 0; i < imx290_modes_num(imx290); i++) { > + unsigned int tmp; > + > + if (v) > + tmp = modes[i].hmax_min - modes[i].width; if (v) return h With the complete series my sensor comes up with controls defined as vertical_blanking 0x009e0901 (int) : min=280 max=261423 step=1 default=280 value=280 horizontal_blanking 0x009e0902 (int) : min=30 max=64255 step=1 default=30 value=30 Set the mode to 1080p and I get vertical_blanking 0x009e0901 (int) : min=45 max=261063 step=1 default=45 value=1169 horizontal_blanking 0x009e0902 (int) : min=280 max=63615 step=1 default=280 value=280 Dave > + else > + tmp = modes[i].vmax_min - modes[i].height; > + > + if (tmp < min) > + min = tmp; > + } > + > + return min; > +} > + > static void imx290_ctrl_update(struct imx290 *imx290, > const struct imx290_mode *mode) > { > > -- > 2.46.0 > >
Hi Dave! On Mon, 2 Sept 2024 at 20:00, Dave Stevenson <dave.stevenson@raspberrypi.com> wrote: > On Mon, 2 Sept 2024 at 16:58, <bbara93@gmail.com> wrote: > > > > From: Benjamin Bara <benjamin.bara@skidata.com> > > > > For now, the driver activates the first mode (1080p) as current active > > mode in probe(). This e.g. means that one cannot set VBLANK below 45 > > (vmax_min - height), although theoretically the minimum is 30 (720p > > mode). Define the absolute possible/supported ranges to have them > > available later. > > Currently the driver will set the ranges for VBLANK and HBLANK > whenever the mode changes. > > How is it helpful to fake these numbers? Seeing as they aren't > reflecting anything useful, they may as well all be 0. > > > Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> > > --- > > Changes since v2: > > - new > > --- > > drivers/media/i2c/imx290.c | 36 ++++++++++++++++++++++++++++++++---- > > 1 file changed, 32 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > > index 1c97f9650eb4..466492bab600 100644 > > --- a/drivers/media/i2c/imx290.c > > +++ b/drivers/media/i2c/imx290.c > > @@ -499,6 +499,10 @@ static const struct imx290_clk_cfg imx290_720p_clock_config[] = { > > }; > > > > /* Mode configs */ > > +#define WIDTH_720P 1280 > > +#define HEIGHT_720P 720 > > +#define MINIMUM_WIDTH WIDTH_720P > > +#define MINIMUM_HEIGHT HEIGHT_720P > > static const struct imx290_mode imx290_modes_2lanes[] = { > > { > > .width = 1920, > > @@ -512,8 +516,8 @@ static const struct imx290_mode imx290_modes_2lanes[] = { > > .clk_cfg = imx290_1080p_clock_config, > > }, > > { > > - .width = 1280, > > - .height = 720, > > + .width = WIDTH_720P, > > + .height = HEIGHT_720P, > > .hmax_min = 3300, > > .vmax_min = 750, > > .link_freq_index = FREQ_INDEX_720P, > > @@ -537,8 +541,8 @@ static const struct imx290_mode imx290_modes_4lanes[] = { > > .clk_cfg = imx290_1080p_clock_config, > > }, > > { > > - .width = 1280, > > - .height = 720, > > + .width = WIDTH_720P, > > + .height = HEIGHT_720P, > > .hmax_min = 3300, > > .vmax_min = 750, > > .link_freq_index = FREQ_INDEX_720P, > > @@ -846,6 +850,30 @@ static const char * const imx290_test_pattern_menu[] = { > > "000/555h Toggle Pattern", > > }; > > > > +/* absolute supported control ranges */ > > +#define HBLANK_MAX (IMX290_HMAX_MAX - MINIMUM_WIDTH) > > +#define VBLANK_MAX (IMX290_VMAX_MAX - MINIMUM_HEIGHT) > > +static unsigned int imx290_get_blank_min(const struct imx290 *imx290, bool v) > > +{ > > This function is never used in this patch. I'm surprised the compiler > doesn't throw an error on a static function not being used. > You first use it in patch 4 "Introduce initial "off" mode & link freq" > > > + const struct imx290_mode *modes = imx290_modes_ptr(imx290); > > + unsigned int min = UINT_MAX; > > + int i; > > + > > + for (i = 0; i < imx290_modes_num(imx290); i++) { > > + unsigned int tmp; > > + > > + if (v) > > + tmp = modes[i].hmax_min - modes[i].width; > > if (v) > return h > > With the complete series my sensor comes up with controls defined as > vertical_blanking 0x009e0901 (int) : min=280 max=261423 step=1 > default=280 value=280 > horizontal_blanking 0x009e0902 (int) : min=30 max=64255 step=1 > default=30 value=30 > > Set the mode to 1080p and I get > vertical_blanking 0x009e0901 (int) : min=45 max=261063 step=1 > default=45 value=1169 > horizontal_blanking 0x009e0902 (int) : min=280 max=63615 step=1 > default=280 value=280 The idea here is to have VBLANK=30 available in the initial "after probe" state of the sensor. VBLANK=30 is a valid value for 720p mode, but it cannot be set after probe, because the driver (not the user) decided that 1080 mode is active. The idea is to relax the ranges while the mode is not set. Once the mode is known, the values are tightened to the real mode-dependent values. Kind regards Benjamin > Dave > > > + else > > + tmp = modes[i].vmax_min - modes[i].height; > > + > > + if (tmp < min) > > + min = tmp; > > + } > > + > > + return min; > > +} > > + > > static void imx290_ctrl_update(struct imx290 *imx290, > > const struct imx290_mode *mode) > > { > > > > -- > > 2.46.0 > > > >
On Mon, Sep 02, 2024 at 09:43:36PM +0200, Benjamin Bara wrote: > Hi Dave! > > On Mon, 2 Sept 2024 at 20:00, Dave Stevenson > <dave.stevenson@raspberrypi.com> wrote: > > On Mon, 2 Sept 2024 at 16:58, <bbara93@gmail.com> wrote: > > > > > > From: Benjamin Bara <benjamin.bara@skidata.com> > > > > > > For now, the driver activates the first mode (1080p) as current active > > > mode in probe(). This e.g. means that one cannot set VBLANK below 45 > > > (vmax_min - height), although theoretically the minimum is 30 (720p > > > mode). Define the absolute possible/supported ranges to have them > > > available later. > > > > Currently the driver will set the ranges for VBLANK and HBLANK > > whenever the mode changes. > > > > How is it helpful to fake these numbers? Seeing as they aren't > > reflecting anything useful, they may as well all be 0. > > > > > Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> > > > --- > > > Changes since v2: > > > - new > > > --- > > > drivers/media/i2c/imx290.c | 36 ++++++++++++++++++++++++++++++++---- > > > 1 file changed, 32 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > > > index 1c97f9650eb4..466492bab600 100644 > > > --- a/drivers/media/i2c/imx290.c > > > +++ b/drivers/media/i2c/imx290.c > > > @@ -499,6 +499,10 @@ static const struct imx290_clk_cfg imx290_720p_clock_config[] = { > > > }; > > > > > > /* Mode configs */ > > > +#define WIDTH_720P 1280 > > > +#define HEIGHT_720P 720 That's pure obfuscation :-) Just use the values directly. > > > +#define MINIMUM_WIDTH WIDTH_720P > > > +#define MINIMUM_HEIGHT HEIGHT_720P Driver-specific macros should have a driver-specific prefix. > > > static const struct imx290_mode imx290_modes_2lanes[] = { > > > { > > > .width = 1920, > > > @@ -512,8 +516,8 @@ static const struct imx290_mode imx290_modes_2lanes[] = { > > > .clk_cfg = imx290_1080p_clock_config, > > > }, > > > { > > > - .width = 1280, > > > - .height = 720, > > > + .width = WIDTH_720P, > > > + .height = HEIGHT_720P, > > > .hmax_min = 3300, > > > .vmax_min = 750, > > > .link_freq_index = FREQ_INDEX_720P, > > > @@ -537,8 +541,8 @@ static const struct imx290_mode imx290_modes_4lanes[] = { > > > .clk_cfg = imx290_1080p_clock_config, > > > }, > > > { > > > - .width = 1280, > > > - .height = 720, > > > + .width = WIDTH_720P, > > > + .height = HEIGHT_720P, > > > .hmax_min = 3300, > > > .vmax_min = 750, > > > .link_freq_index = FREQ_INDEX_720P, > > > @@ -846,6 +850,30 @@ static const char * const imx290_test_pattern_menu[] = { > > > "000/555h Toggle Pattern", > > > }; > > > > > > +/* absolute supported control ranges */ > > > +#define HBLANK_MAX (IMX290_HMAX_MAX - MINIMUM_WIDTH) > > > +#define VBLANK_MAX (IMX290_VMAX_MAX - MINIMUM_HEIGHT) Here too. > > > +static unsigned int imx290_get_blank_min(const struct imx290 *imx290, bool v) > > > +{ > > > > This function is never used in this patch. I'm surprised the compiler > > doesn't throw an error on a static function not being used. > > You first use it in patch 4 "Introduce initial "off" mode & link freq" > > > > > + const struct imx290_mode *modes = imx290_modes_ptr(imx290); > > > + unsigned int min = UINT_MAX; > > > + int i; > > > + > > > + for (i = 0; i < imx290_modes_num(imx290); i++) { > > > + unsigned int tmp; > > > + > > > + if (v) > > > + tmp = modes[i].hmax_min - modes[i].width; > > > > if (v) > > return h > > > > With the complete series my sensor comes up with controls defined as > > vertical_blanking 0x009e0901 (int) : min=280 max=261423 step=1 > > default=280 value=280 > > horizontal_blanking 0x009e0902 (int) : min=30 max=64255 step=1 > > default=30 value=30 > > > > Set the mode to 1080p and I get > > vertical_blanking 0x009e0901 (int) : min=45 max=261063 step=1 > > default=45 value=1169 > > horizontal_blanking 0x009e0902 (int) : min=280 max=63615 step=1 > > default=280 value=280 > > The idea here is to have VBLANK=30 available in the initial "after > probe" state of the sensor. VBLANK=30 is a valid value for 720p mode, > but it cannot be set after probe, because the driver (not the user) > decided that 1080 mode is active. The idea is to relax the ranges while > the mode is not set. Once the mode is known, the values are tightened > to the real mode-dependent values. > > Kind regards > Benjamin > > > Dave > > > > > + else > > > + tmp = modes[i].vmax_min - modes[i].height; > > > + > > > + if (tmp < min) > > > + min = tmp; > > > + } > > > + > > > + return min; > > > +} > > > + > > > static void imx290_ctrl_update(struct imx290 *imx290, > > > const struct imx290_mode *mode) > > > {
Hi Laurent! On Mon, 2 Sept 2024 at 22:06, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Mon, Sep 02, 2024 at 09:43:36PM +0200, Benjamin Bara wrote: > > On Mon, 2 Sept 2024 at 20:00, Dave Stevenson > > <dave.stevenson@raspberrypi.com> wrote: > > > On Mon, 2 Sept 2024 at 16:58, <bbara93@gmail.com> wrote: > > > > > > > > From: Benjamin Bara <benjamin.bara@skidata.com> > > > > > > > > For now, the driver activates the first mode (1080p) as current active > > > > mode in probe(). This e.g. means that one cannot set VBLANK below 45 > > > > (vmax_min - height), although theoretically the minimum is 30 (720p > > > > mode). Define the absolute possible/supported ranges to have them > > > > available later. > > > > > > Currently the driver will set the ranges for VBLANK and HBLANK > > > whenever the mode changes. > > > > > > How is it helpful to fake these numbers? Seeing as they aren't > > > reflecting anything useful, they may as well all be 0. > > > > > > > Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> > > > > --- > > > > Changes since v2: > > > > - new > > > > --- > > > > drivers/media/i2c/imx290.c | 36 ++++++++++++++++++++++++++++++++---- > > > > 1 file changed, 32 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > > > > index 1c97f9650eb4..466492bab600 100644 > > > > --- a/drivers/media/i2c/imx290.c > > > > +++ b/drivers/media/i2c/imx290.c > > > > @@ -499,6 +499,10 @@ static const struct imx290_clk_cfg imx290_720p_clock_config[] = { > > > > }; > > > > > > > > /* Mode configs */ > > > > +#define WIDTH_720P 1280 > > > > +#define HEIGHT_720P 720 > > That's pure obfuscation :-) Just use the values directly. > > > > > +#define MINIMUM_WIDTH WIDTH_720P > > > > +#define MINIMUM_HEIGHT HEIGHT_720P > > Driver-specific macros should have a driver-specific prefix. > > > > > static const struct imx290_mode imx290_modes_2lanes[] = { > > > > { > > > > .width = 1920, > > > > @@ -512,8 +516,8 @@ static const struct imx290_mode imx290_modes_2lanes[] = { > > > > .clk_cfg = imx290_1080p_clock_config, > > > > }, > > > > { > > > > - .width = 1280, > > > > - .height = 720, > > > > + .width = WIDTH_720P, > > > > + .height = HEIGHT_720P, > > > > .hmax_min = 3300, > > > > .vmax_min = 750, > > > > .link_freq_index = FREQ_INDEX_720P, > > > > @@ -537,8 +541,8 @@ static const struct imx290_mode imx290_modes_4lanes[] = { > > > > .clk_cfg = imx290_1080p_clock_config, > > > > }, > > > > { > > > > - .width = 1280, > > > > - .height = 720, > > > > + .width = WIDTH_720P, > > > > + .height = HEIGHT_720P, > > > > .hmax_min = 3300, > > > > .vmax_min = 750, > > > > .link_freq_index = FREQ_INDEX_720P, > > > > @@ -846,6 +850,30 @@ static const char * const imx290_test_pattern_menu[] = { > > > > "000/555h Toggle Pattern", > > > > }; > > > > > > > > +/* absolute supported control ranges */ > > > > +#define HBLANK_MAX (IMX290_HMAX_MAX - MINIMUM_WIDTH) > > > > +#define VBLANK_MAX (IMX290_VMAX_MAX - MINIMUM_HEIGHT) > > Here too. Thanks for the feedback but I will drop this patch. As we need to decide on a link frequency, we need to decide on one of the two "all pixel" modes the imx290 supports and therefore already know the mode-specific, tight ranges starting from probe time. Kind regards Benjamin > > > > +static unsigned int imx290_get_blank_min(const struct imx290 *imx290, bool v) > > > > +{ > > > > > > This function is never used in this patch. I'm surprised the compiler > > > doesn't throw an error on a static function not being used. > > > You first use it in patch 4 "Introduce initial "off" mode & link freq" > > > > > > > + const struct imx290_mode *modes = imx290_modes_ptr(imx290); > > > > + unsigned int min = UINT_MAX; > > > > + int i; > > > > + > > > > + for (i = 0; i < imx290_modes_num(imx290); i++) { > > > > + unsigned int tmp; > > > > + > > > > + if (v) > > > > + tmp = modes[i].hmax_min - modes[i].width; > > > > > > if (v) > > > return h > > > > > > With the complete series my sensor comes up with controls defined as > > > vertical_blanking 0x009e0901 (int) : min=280 max=261423 step=1 > > > default=280 value=280 > > > horizontal_blanking 0x009e0902 (int) : min=30 max=64255 step=1 > > > default=30 value=30 > > > > > > Set the mode to 1080p and I get > > > vertical_blanking 0x009e0901 (int) : min=45 max=261063 step=1 > > > default=45 value=1169 > > > horizontal_blanking 0x009e0902 (int) : min=280 max=63615 step=1 > > > default=280 value=280 > > > > The idea here is to have VBLANK=30 available in the initial "after > > probe" state of the sensor. VBLANK=30 is a valid value for 720p mode, > > but it cannot be set after probe, because the driver (not the user) > > decided that 1080 mode is active. The idea is to relax the ranges while > > the mode is not set. Once the mode is known, the values are tightened > > to the real mode-dependent values. > > > > Kind regards > > Benjamin > > > > > Dave > > > > > > > + else > > > > + tmp = modes[i].vmax_min - modes[i].height; > > > > + > > > > + if (tmp < min) > > > > + min = tmp; > > > > + } > > > > + > > > > + return min; > > > > +} > > > > + > > > > static void imx290_ctrl_update(struct imx290 *imx290, > > > > const struct imx290_mode *mode) > > > > { > > -- > Regards, > > Laurent Pinchart
Hi Dave! On Mon, 2 Sept 2024 at 20:00, Dave Stevenson <dave.stevenson@raspberrypi.com> wrote: > On Mon, 2 Sept 2024 at 16:58, <bbara93@gmail.com> wrote: > > > > From: Benjamin Bara <benjamin.bara@skidata.com> > > > > For now, the driver activates the first mode (1080p) as current active > > mode in probe(). This e.g. means that one cannot set VBLANK below 45 > > (vmax_min - height), although theoretically the minimum is 30 (720p > > mode). Define the absolute possible/supported ranges to have them > > available later. > > Currently the driver will set the ranges for VBLANK and HBLANK > whenever the mode changes. > > How is it helpful to fake these numbers? Seeing as they aren't > reflecting anything useful, they may as well all be 0. > > > Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com> > > --- > > Changes since v2: > > - new > > --- > > drivers/media/i2c/imx290.c | 36 ++++++++++++++++++++++++++++++++---- > > 1 file changed, 32 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > > index 1c97f9650eb4..466492bab600 100644 > > --- a/drivers/media/i2c/imx290.c > > +++ b/drivers/media/i2c/imx290.c > > @@ -499,6 +499,10 @@ static const struct imx290_clk_cfg imx290_720p_clock_config[] = { > > }; > > > > /* Mode configs */ > > +#define WIDTH_720P 1280 > > +#define HEIGHT_720P 720 > > +#define MINIMUM_WIDTH WIDTH_720P > > +#define MINIMUM_HEIGHT HEIGHT_720P > > static const struct imx290_mode imx290_modes_2lanes[] = { > > { > > .width = 1920, > > @@ -512,8 +516,8 @@ static const struct imx290_mode imx290_modes_2lanes[] = { > > .clk_cfg = imx290_1080p_clock_config, > > }, > > { > > - .width = 1280, > > - .height = 720, > > + .width = WIDTH_720P, > > + .height = HEIGHT_720P, > > .hmax_min = 3300, > > .vmax_min = 750, > > .link_freq_index = FREQ_INDEX_720P, > > @@ -537,8 +541,8 @@ static const struct imx290_mode imx290_modes_4lanes[] = { > > .clk_cfg = imx290_1080p_clock_config, > > }, > > { > > - .width = 1280, > > - .height = 720, > > + .width = WIDTH_720P, > > + .height = HEIGHT_720P, > > .hmax_min = 3300, > > .vmax_min = 750, > > .link_freq_index = FREQ_INDEX_720P, > > @@ -846,6 +850,30 @@ static const char * const imx290_test_pattern_menu[] = { > > "000/555h Toggle Pattern", > > }; > > > > +/* absolute supported control ranges */ > > +#define HBLANK_MAX (IMX290_HMAX_MAX - MINIMUM_WIDTH) > > +#define VBLANK_MAX (IMX290_VMAX_MAX - MINIMUM_HEIGHT) > > +static unsigned int imx290_get_blank_min(const struct imx290 *imx290, bool v) > > +{ > > This function is never used in this patch. I'm surprised the compiler > doesn't throw an error on a static function not being used. > You first use it in patch 4 "Introduce initial "off" mode & link freq" I think I couldn't really transport my intention behind this function and going to try to explain my thought process a little bit: My basic understanding of sensors and blanking times is that the sensors cannot "just forward image data" but also need some time for internal stuff. My expectation would be that this time should be the same for two different modes (720p and 1080p) of the same type ("all pixel readout"). I would even expect that in this type of mode, most of the "internal time" is used for transferring data, therefore I would also expect that the minimum blanking times for 720p to be lower (or at least equal) than 1080p. This would make my intention quite easy: Just activate the lowest resolution during probe, because it has the widest allowed blanking time ranges, and then we can "freely configure", independent of the mode later used. However, this is not the case here: The minimum VBLANK of the 1080p mode is lower than the minimum VBLANK of the 720p mode. Together with HBLANK_min_720p < HBLANK_min_1080p, we need a "virtual, invalid" mode to support both minimas. What would make things simpler here would be allowing us to control the total times instead of the blanking times. In this case, we can directly use the minimum defined {H,V}MAX of the mode and default to the mode with the lowest possible resolution instead. This would enable a freely configurable frame duration with full supported ranges and valid modes. Kind regards Benjamin > > + const struct imx290_mode *modes = imx290_modes_ptr(imx290); > > + unsigned int min = UINT_MAX; > > + int i; > > + > > + for (i = 0; i < imx290_modes_num(imx290); i++) { > > + unsigned int tmp; > > + > > + if (v) > > + tmp = modes[i].hmax_min - modes[i].width; > > if (v) > return h > > With the complete series my sensor comes up with controls defined as > vertical_blanking 0x009e0901 (int) : min=280 max=261423 step=1 > default=280 value=280 > horizontal_blanking 0x009e0902 (int) : min=30 max=64255 step=1 > default=30 value=30 > > Set the mode to 1080p and I get > vertical_blanking 0x009e0901 (int) : min=45 max=261063 step=1 > default=45 value=1169 > horizontal_blanking 0x009e0902 (int) : min=280 max=63615 step=1 > default=280 value=280 > > Dave > > > + else > > + tmp = modes[i].vmax_min - modes[i].height; > > + > > + if (tmp < min) > > + min = tmp; > > + } > > + > > + return min; > > +} > > + > > static void imx290_ctrl_update(struct imx290 *imx290, > > const struct imx290_mode *mode) > > { > > > > -- > > 2.46.0 > > > >
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 1c97f9650eb4..466492bab600 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -499,6 +499,10 @@ static const struct imx290_clk_cfg imx290_720p_clock_config[] = { }; /* Mode configs */ +#define WIDTH_720P 1280 +#define HEIGHT_720P 720 +#define MINIMUM_WIDTH WIDTH_720P +#define MINIMUM_HEIGHT HEIGHT_720P static const struct imx290_mode imx290_modes_2lanes[] = { { .width = 1920, @@ -512,8 +516,8 @@ static const struct imx290_mode imx290_modes_2lanes[] = { .clk_cfg = imx290_1080p_clock_config, }, { - .width = 1280, - .height = 720, + .width = WIDTH_720P, + .height = HEIGHT_720P, .hmax_min = 3300, .vmax_min = 750, .link_freq_index = FREQ_INDEX_720P, @@ -537,8 +541,8 @@ static const struct imx290_mode imx290_modes_4lanes[] = { .clk_cfg = imx290_1080p_clock_config, }, { - .width = 1280, - .height = 720, + .width = WIDTH_720P, + .height = HEIGHT_720P, .hmax_min = 3300, .vmax_min = 750, .link_freq_index = FREQ_INDEX_720P, @@ -846,6 +850,30 @@ static const char * const imx290_test_pattern_menu[] = { "000/555h Toggle Pattern", }; +/* absolute supported control ranges */ +#define HBLANK_MAX (IMX290_HMAX_MAX - MINIMUM_WIDTH) +#define VBLANK_MAX (IMX290_VMAX_MAX - MINIMUM_HEIGHT) +static unsigned int imx290_get_blank_min(const struct imx290 *imx290, bool v) +{ + const struct imx290_mode *modes = imx290_modes_ptr(imx290); + unsigned int min = UINT_MAX; + int i; + + for (i = 0; i < imx290_modes_num(imx290); i++) { + unsigned int tmp; + + if (v) + tmp = modes[i].hmax_min - modes[i].width; + else + tmp = modes[i].vmax_min - modes[i].height; + + if (tmp < min) + min = tmp; + } + + return min; +} + static void imx290_ctrl_update(struct imx290 *imx290, const struct imx290_mode *mode) {