Hello,
I'm trying to get camera on HP Spectre x360 14-eu0xxx (2024) laptop to work.
I was able to make main sensor driver (ov08x40) to play nice with IPU6,
INT3472 and libcamera-SoftISP and the resulting image quality is absolutely
usable and even surprisingly good.
This laptop also uses this "MIPI aggregator".
> Hi,
>
> On 10/7/23 04:12, Hao Yao wrote:
> > Handshake pin is used for Lattice MIPI aggregator to enable the
> > camera sensor. After pulled up, recommend to wail ~250ms to get
> > everything ready.
>
> If this is a pin on the "Lattice MIPI aggregator" and
> not on the sensor itself then this really should be
> modeled as such and should not be registered as a GPIO
> consumed by the sensor since the actual sensor does not
> have a handshake pin at all.
>
> Also we really don't want to need to patch all involved
> sensor drivers to toggle a handshake pin, especially since
> the sensor itself does not physically have this pin.
>
> Can you explain a bit more:
>
> 1. What the "Lattice MIPI aggregator" is.
> 2. What its functions are, does this control reset + pwdn
> GPIOs for the sensor? Voltages to the sensor? Clk
> to the sensor ?
It acts like MIPI switch as no MIPI data gets from the sensor to IPU6 if
handshake signal is not asserted. Eventually IPU6 times out with "start stream
of firmware failed" message. Any further attempts to start streaming lead to
a panic.
I2C communication is not affected by the handshake signal but it looks like
reset signal is also going through this "MIPI aggregator" as it takes about
150ms for the sensor to reliably start responding via I2C after the reset
is deasserted. It should be about few ms if the reset signal was connected to
the sensor directly.
> 3. How the aggregator is connected to both the main
> CPU/SoC as well as how it is connected to the sensor ?
> Some example diagram would be really helpful here.
>
> Then with this info in hand we can try to come up
> with a way how to model this.
>
> Assuming this controls the entire power-up sequence
> for the sensor then I think it could be modelled
> as a GPIO regulator. This also allows making the
> regulator core take care of the necessary delay
> between setting the GPIO and trying to talk to
> the sensor.
Are there any updates on how this signal should be implemented? For now I'm
just applying this patch and asserting it from the sensor driver.
Regards
>
> Regards,
>
> Hans
>
>
>
> >
> > Signed-off-by: Hao Yao <hao.yao@xxxxxxxxx>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> > drivers/platform/x86/intel/int3472/common.h | 1 +
> > drivers/platform/x86/intel/int3472/discrete.c | 5 +++++
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/platform/x86/intel/int3472/common.h b/drivers/platform/x86/intel/int3472/common.h
> > index 655ae3ec0593..3ad4c72afb45 100644
> > --- a/drivers/platform/x86/intel/int3472/common.h
> > +++ b/drivers/platform/x86/intel/int3472/common.h
> > @@ -23,6 +23,7 @@
> > #define INT3472_GPIO_TYPE_POWER_ENABLE 0x0b
> > #define INT3472_GPIO_TYPE_CLK_ENABLE 0x0c
> > #define INT3472_GPIO_TYPE_PRIVACY_LED 0x0d
> > +#define INT3472_GPIO_TYPE_HANDSHAKE 0x12
> >
> > #define INT3472_PDEV_MAX_NAME_LEN 23
> > #define INT3472_MAX_SENSOR_GPIOS 3
> > diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> > index b644ce65c990..4753161b4080 100644
> > --- a/drivers/platform/x86/intel/int3472/discrete.c
> > +++ b/drivers/platform/x86/intel/int3472/discrete.c
> > @@ -111,6 +111,10 @@ static void int3472_get_func_and_polarity(u8 type, const char **func, u32 *polar
> > *func = "power-enable";
> > *polarity = GPIO_ACTIVE_HIGH;
> > break;
> > + case INT3472_GPIO_TYPE_HANDSHAKE:
> > + *func = "handshake";
> > + *polarity = GPIO_ACTIVE_HIGH;
> > + break;
> > default:
> > *func = "unknown";
> > *polarity = GPIO_ACTIVE_HIGH;
> > @@ -201,6 +205,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
> > switch (type) {
> > case INT3472_GPIO_TYPE_RESET:
> > case INT3472_GPIO_TYPE_POWERDOWN:
> > + case INT3472_GPIO_TYPE_HANDSHAKE:
> > ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, func, polarity);
> > if (ret)
> > err_msg = "Failed to map GPIO pin to sensor\n";
>
