Hi Benjamin
On Mon, 2 Sept 2024 at 16:58, <bbara93@xxxxxxxxx> wrote:
>
> From: Benjamin Bara <benjamin.bara@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> ---
> 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
>
>
