Re: [PATCH] media: ipu-bridge: check all DMI entries when overriding sensor rotation

Hi José,

Thanks for the patch.

On Mon, Aug 10, 2026 at 01:38:45AM +0200, José María Martín wrote:
> A machine can have more than one sensor whose rotation needs to be
> overridden, which takes one upside_down_sensor_dmi_ids[] entry per
> sensor, all sharing the same DMI match but with different ACPI HIDs in
> driver_data.
> 
> ipu_bridge_parse_rotation() uses dmi_first_match(), which always stops
> at the first entry matching the running machine, so any further entry
> for the same machine is unreachable and only one sensor per machine can
> ever be corrected.
> 
> No machine in the mainline table currently lists two sensors, but the
> Microsoft Surface Pro 9 has both its front (OVTI5693) and rear
> (OVTID858) cameras mounted upside down while its SSDB reports degree=0
> for both; the linux-surface downstream kernel carries such a pair of
> entries and hits this limitation.

This paragraph would better fit for a cover letter.

> 
> Walk the whole table and match every entry for the running machine
> against the sensor's ACPI HID instead.
> 
> Tested on a Surface Pro 9 (6.19-based linux-surface kernel with this
> change backported): both cameras report Rotation = 180 and render
> upright in libcamera clients.

Ditto.

> 
> Signed-off-by: José María Martín <[email protected]>

I think a Fixes: tag would be appropriate.

> ---
>  drivers/media/pci/intel/ipu-bridge.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
> index 88581a4..75db4cb 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -298,9 +298,18 @@ static u32 ipu_bridge_parse_rotation(struct acpi_device *adev,
>  {
>  	const struct dmi_system_id *dmi_id;
>  
> -	dmi_id = dmi_first_match(upside_down_sensor_dmi_ids);
> -	if (dmi_id && acpi_dev_hid_match(adev, dmi_id->driver_data))
> -		return 180;
> +	/*
> +	 * A machine can have more than one sensor whose rotation must be
> +	 * overridden (e.g. both the front and the rear camera), listed as
> +	 * one entry per sensor sharing the same DMI match.
> +	 * dmi_first_match() only ever returns the first matching entry, so
> +	 * walk the whole table and match each candidate against the
> +	 * sensor's ACPI HID.
> +	 */

I don't think such an elaborate comment is needed, either make it shorter
or drop it altogether.

> +	for (dmi_id = dmi_first_match(upside_down_sensor_dmi_ids); dmi_id;
> +	     dmi_id = dmi_first_match(dmi_id + 1))
> +		if (acpi_dev_hid_match(adev, dmi_id->driver_data))
> +			return 180;
>  
>  	switch (ssdb->degree) {
>  	case IPU_SENSOR_ROTATION_NORMAL:

-- 
Regards,

Sakari Ailus