[28/28] media: atomisp: gmin_platform: Add Lenovo Ideapad Miix 310 gmin_vars

Message ID 20230401145926.596216-29-hdegoede@redhat.com (mailing list archive)
State Accepted
Headers
Series media: atomisp: Further sensor rework + exotic features removal |

Commit Message

Hans de Goede April 1, 2023, 2:59 p.m. UTC
  The _DSM used to get sensor variables like CsiPort returns the wrong
csi-port for the front OV2680 sensor on the Lenovo Ideapad Miix 310
add a gmin_vars DMI quirk / override setting the right CsiPort.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../media/atomisp/pci/atomisp_gmin_platform.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
  

Comments

Andy Shevchenko April 2, 2023, 7:12 p.m. UTC | #1
On Sat, Apr 1, 2023 at 5:00 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The _DSM used to get sensor variables like CsiPort returns the wrong
> csi-port for the front OV2680 sensor on the Lenovo Ideapad Miix 310
> add a gmin_vars DMI quirk / override setting the right CsiPort.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../media/atomisp/pci/atomisp_gmin_platform.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> index f83de0ffaf16..efcfc133311f 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> @@ -304,7 +304,17 @@ static struct gmin_cfg_var surface3_vars[] = {
>         {},
>  };
>
> +static struct gmin_cfg_var lenovo_ideapad_miix_310_vars[] = {
> +       /* _DSM contains the wrong CsiPort! */
> +       { "OVTI2680:01_CsiPort", "0" },
> +       {}
> +};
> +
>  static const struct dmi_system_id gmin_vars[] = {
> +       /*
> +        * These DMI ids where present when the atomisp driver was merged into

IDs
were ?

> +        * drivers/staging and it is unclear if they are really necessary.
> +        */

MRD7 and FFD8 are the reference designs. At least MRD7 is what we have
in the lab and we can run some tests there. That's, for example, one
which I used to run atomisp before it got removed from the kernel a
few years ago.

>         {
>                 .ident = "BYT-T FFD8",
>                 .matches = {
> @@ -341,6 +351,7 @@ static const struct dmi_system_id gmin_vars[] = {
>                 },
>                 .driver_data = i8880_vars,
>         },
> +       /* Later added DMI ids, these are confirmed to really be necessary! */
>         {
>                 .ident = "Surface 3",
>                 .matches = {
> @@ -348,6 +359,14 @@ static const struct dmi_system_id gmin_vars[] = {
>                 },
>                 .driver_data = surface3_vars,
>         },
> +       {
> +               .ident = "Lenovo Ideapad Miix 310",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +                       DMI_MATCH(DMI_PRODUCT_VERSION, "MIIX 310-10"),
> +               },
> +               .driver_data = lenovo_ideapad_miix_310_vars,
> +       },
>         {}
>  };
>
> --
> 2.39.1
>
  
Hans de Goede April 9, 2023, 1:01 p.m. UTC | #2
Hi,

On 4/2/23 21:12, Andy Shevchenko wrote:
> On Sat, Apr 1, 2023 at 5:00 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> The _DSM used to get sensor variables like CsiPort returns the wrong
>> csi-port for the front OV2680 sensor on the Lenovo Ideapad Miix 310
>> add a gmin_vars DMI quirk / override setting the right CsiPort.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  .../media/atomisp/pci/atomisp_gmin_platform.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
>> index f83de0ffaf16..efcfc133311f 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
>> @@ -304,7 +304,17 @@ static struct gmin_cfg_var surface3_vars[] = {
>>         {},
>>  };
>>
>> +static struct gmin_cfg_var lenovo_ideapad_miix_310_vars[] = {
>> +       /* _DSM contains the wrong CsiPort! */
>> +       { "OVTI2680:01_CsiPort", "0" },
>> +       {}
>> +};
>> +
>>  static const struct dmi_system_id gmin_vars[] = {
>> +       /*
>> +        * These DMI ids where present when the atomisp driver was merged into
> 
> IDs
> were ?

Ack fixed.

>> +        * drivers/staging and it is unclear if they are really necessary.
>> +        */
> 
> MRD7 and FFD8 are the reference designs. At least MRD7 is what we have
> in the lab and we can run some tests there. That's, for example, one
> which I used to run atomisp before it got removed from the kernel a
> few years ago.

Right, but do these need the quirks ? Or is our auto-detection code
good enough to get the right values ?

Regards,

Hans



> 
>>         {
>>                 .ident = "BYT-T FFD8",
>>                 .matches = {
>> @@ -341,6 +351,7 @@ static const struct dmi_system_id gmin_vars[] = {
>>                 },
>>                 .driver_data = i8880_vars,
>>         },
>> +       /* Later added DMI ids, these are confirmed to really be necessary! */
>>         {
>>                 .ident = "Surface 3",
>>                 .matches = {
>> @@ -348,6 +359,14 @@ static const struct dmi_system_id gmin_vars[] = {
>>                 },
>>                 .driver_data = surface3_vars,
>>         },
>> +       {
>> +               .ident = "Lenovo Ideapad Miix 310",
>> +               .matches = {
>> +                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> +                       DMI_MATCH(DMI_PRODUCT_VERSION, "MIIX 310-10"),
>> +               },
>> +               .driver_data = lenovo_ideapad_miix_310_vars,
>> +       },
>>         {}
>>  };
>>
>> --
>> 2.39.1
>>
> 
>
  
Andy Shevchenko April 10, 2023, 9:35 a.m. UTC | #3
On Sun, Apr 9, 2023 at 4:01 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 4/2/23 21:12, Andy Shevchenko wrote:
> > On Sat, Apr 1, 2023 at 5:00 PM Hans de Goede <hdegoede@redhat.com> wrote:

...

> > MRD7 and FFD8 are the reference designs. At least MRD7 is what we have
> > in the lab and we can run some tests there. That's, for example, one
> > which I used to run atomisp before it got removed from the kernel a
> > few years ago.
>
> Right, but do these need the quirks ? Or is our auto-detection code
> good enough to get the right values ?

I will try to allocate some time to check it against the latest
AtomISP code. Do you have a branch where you have been collecting the
all related patches?
  

Patch

diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
index f83de0ffaf16..efcfc133311f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
@@ -304,7 +304,17 @@  static struct gmin_cfg_var surface3_vars[] = {
 	{},
 };
 
+static struct gmin_cfg_var lenovo_ideapad_miix_310_vars[] = {
+	/* _DSM contains the wrong CsiPort! */
+	{ "OVTI2680:01_CsiPort", "0" },
+	{}
+};
+
 static const struct dmi_system_id gmin_vars[] = {
+	/*
+	 * These DMI ids where present when the atomisp driver was merged into
+	 * drivers/staging and it is unclear if they are really necessary.
+	 */
 	{
 		.ident = "BYT-T FFD8",
 		.matches = {
@@ -341,6 +351,7 @@  static const struct dmi_system_id gmin_vars[] = {
 		},
 		.driver_data = i8880_vars,
 	},
+	/* Later added DMI ids, these are confirmed to really be necessary! */
 	{
 		.ident = "Surface 3",
 		.matches = {
@@ -348,6 +359,14 @@  static const struct dmi_system_id gmin_vars[] = {
 		},
 		.driver_data = surface3_vars,
 	},
+	{
+		.ident = "Lenovo Ideapad Miix 310",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "MIIX 310-10"),
+		},
+		.driver_data = lenovo_ideapad_miix_310_vars,
+	},
 	{}
 };