Re: [PATCH v2 3/4] media: hi846: Add 6MP and 8MP modes support

[Pengyu Luo]

On Wed, May 6, 2026 at 8:51 PM Sebastian Krzyszkowiak
<sebastian.krzyszkowiak@xxxxxxx> wrote:
>
> On piątek, 1 maja 2026 11:54:32 czas środkowoeuropejski letni Pengyu Luo
> wrote:
> > Hi846 is an 8MP sensor, but the upstream driver has only supported 2MP
> > mode for years. This patch adds 6MP and 8MP modes to maximize sensor
> > utilization.
> >
> > Note that these modes require 4-lane MIPI CSI-2, as the downstream
> > driver only exposes 2MP, 6MP, and 8MP configurations in 4-lane
> > operation on the target device. The register sequences are extracted
> > from the downstream Windows driver.
>
> Has this been tested in this form at all? I tried, failed and looking at the
> driver I don't see how could it ever end up using these modes in its current
> state. It can only cause troubles when used with 2 lanes, while 4 lanes are
> completely broken.
>

I may have messed up. I tested it with two more patches. The first
patch removes 480P and 720P, the second uses a new 4-lane init list.(I
just sincerely followed the downstream) Since I didn't think much, I
thought removing modes would not break things, I just tested without
the second patch, so it works for me.

> The driver defaults to its first supported mode, which happens to be a 640x480
> mode that only defines its register list for 2 lanes. hi846_set_format was
> supposedly meant to check whether the mode returned by v4l2_find_nearest_size
> is compatible with the used lane count, but it does so too early - it actually
> checks it against the already set mode rather than the one it's about to set.
> This means that you're never going to be able to set any valid mode when using
> 4 lanes without fixing this first.
>
> And even if this was fixed, with 2 lanes this makes some calls from userspace
> that previously succeeded now fail when v4l2_find_nearest_size happens to match
> a mode that's only supported with 4 lanes. v4l2_find_nearest_size would have to
> be fed with already filtered list of modes to fix that (which would actually
> make the check mentioned above unnecessary).
>
> Of course these are preexisting issues in the driver, but they make it
> impossible to actually use what's being added in this patch and to add these
> modes without causing regressions. So far these didn't matter as this driver
> was only ever used with 2 lanes, but they have to be fixed first before adding
> any actual 4 lane usage or modes that require 4 lanes.
>

Thanks for your feedback. I think we can use specific modes for
different lane modes. The snippet like

    hi846->supported_modes = supported_modes;
    hi846->num_modes = ARRAY_SIZE(supported_modes) - 2;
    if (hi846->nr_lanes == 4) {
        hi846->supported_modes = supported_modes + 1;
        hi846->num_modes = ARRAY_SIZE(supported_modes) - 1;
    }

Best wishes,
Pengyu

> S.
>
>