Re: [Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block (v3)
On Fri, 10 Jan 2020, Vivek Kasireddy wrote:
> Parsing the i2c element is mainly done to transfer the payload from the
> MIPI sequence block to the relevant slave device. In some cases, the
> commands that are part of the payload can be used to turn on the backlight.
>
> This patch is actually a refactored version of this old patch:
> https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
>
> In addition to the refactoring, the original patch is augmented by looking up
> the i2c bus from ACPI NS instead of relying on the bus number provided
> in the VBT.
All this acpi business in the patch will fail the build on
CONFIG_ACPI=n.
BR,
Jani.
>
> This patch was tested on Aava Mobile's Inari 10 tablet. It enabled
> turning on the backlight by transfering the payload to the device.
>
> v2:
> - Add DRM_DEV_ERROR for invalid adapter and failed transfer and also
> drop the DRM_DEBUG that existed originally. (Hans)
> - Add two gotos instead of one to clean things up properly.
>
> v3:
> - Identify the device on which this patch was tested in the commit
> message (Ville)
>
> Cc: Hans de Goede
> Cc: Nabendu Maiti
> Cc: Matt Roper
> Cc: Bob Paauwe
> Cc: Ville Syrjälä
> Reviewed-by: Hans de Goede
> Signed-off-by: Vivek Kasireddy
> ---
> drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 99 +++-
> 2 files changed, 100 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
> b/drivers/gpu/drm/i915/display/intel_dsi.h
> index 7481a5aa3084..6cef1356b4e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
> @@ -69,6 +69,9 @@ struct intel_dsi {
> /* number of DSI lanes */
> unsigned int lane_count;
>
> + /* i2c bus associated with the slave device */
> + int i2c_bus_num;
> +
> /*
>* video mode pixel format
>*
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index 0032161e0f76..89fb0d90b694 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -86,6 +86,12 @@ static struct gpio_map vlv_gpio_table[] = {
> { VLV_GPIO_NC_11_PANEL1_BKLTCTL },
> };
>
> +struct i2c_adapter_lookup {
> + u16 slave_addr;
> + struct intel_dsi *intel_dsi;
> + acpi_handle dev_handle;
> +};
> +
> #define CHV_GPIO_IDX_START_N 0
> #define CHV_GPIO_IDX_START_E 73
> #define CHV_GPIO_IDX_START_SW100
> @@ -378,11 +384,98 @@ static const u8 *mipi_exec_gpio(struct intel_dsi
> *intel_dsi, const u8 *data)
> return data;
> }
>
> +static int i2c_adapter_lookup(struct acpi_resource *ares, void *data)
> +{
> + struct i2c_adapter_lookup *lookup = data;
> + struct intel_dsi *intel_dsi = lookup->intel_dsi;
> + struct acpi_resource_i2c_serialbus *sb;
> + struct i2c_adapter *adapter;
> + acpi_handle adapter_handle;
> + acpi_status status;
> +
> + if (intel_dsi->i2c_bus_num >= 0 ||
> + !i2c_acpi_get_i2c_resource(ares, &sb))
> + return 1;
> +
> + if (lookup->slave_addr != sb->slave_address)
> + return 1;
> +
> + status = acpi_get_handle(lookup->dev_handle,
> + sb->resource_source.string_ptr,
> + &adapter_handle);
> + if (ACPI_FAILURE(status))
> + return 1;
> +
> + adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
> + if (adapter)
> + intel_dsi->i2c_bus_num = adapter->nr;
> +
> + return 1;
> +}
> +
> static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
> {
> - DRM_DEBUG_KMS("Skipping I2C element execution\n");
> + struct drm_device *drm_dev = intel_dsi->base.base.dev;
> + struct device *dev = &drm_dev->pdev->dev;
> + struct i2c_adapter *adapter;
> + struct acpi_device *acpi_dev;
> + struct list_head resource_list;
> + struct i2c_adapter_lookup lookup;
> + struct i2c_msg msg;
> + int ret;
> + u8 vbt_i2c_bus_num = *(data + 2);
> + u16 slave_addr = *(u16 *)(data + 3);
> + u8 reg_offset = *(data + 5);
> + u8 payload_size = *(data + 6);
> + u8 *payload_data;
> +
> + if (intel_dsi->i2c_bus_num < 0) {
> + intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
> +
> + acpi_dev = ACPI_COMPANION(dev);
> + if (acpi_dev) {
> + memset(&lookup, 0, sizeof(lookup));
> + lookup.slave_addr = slave_addr;
> + lookup.intel_dsi = intel_dsi;
> + lookup.dev_handle = acpi_device_handle(acpi_dev);
> +
> + INIT_LIST_HEAD(&resource_list);
> + acpi_dev_get_resources(acpi_dev, &resource_list,
> +i2c_adapter_lookup,
> +
Re: [Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block (v3)
On Fri, Jan 10, 2020 at 10:11:23AM -0800, Vivek Kasireddy wrote:
> Parsing the i2c element is mainly done to transfer the payload from the
> MIPI sequence block to the relevant slave device. In some cases, the
> commands that are part of the payload can be used to turn on the backlight.
>
> This patch is actually a refactored version of this old patch:
> https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
>
> In addition to the refactoring, the original patch is augmented by looking up
> the i2c bus from ACPI NS instead of relying on the bus number provided
> in the VBT.
>
> This patch was tested on Aava Mobile's Inari 10 tablet. It enabled
> turning on the backlight by transfering the payload to the device.
>
> v2:
> - Add DRM_DEV_ERROR for invalid adapter and failed transfer and also
> drop the DRM_DEBUG that existed originally. (Hans)
> - Add two gotos instead of one to clean things up properly.
>
> v3:
> - Identify the device on which this patch was tested in the commit
> message (Ville)
>
> Cc: Hans de Goede
> Cc: Nabendu Maiti
> Cc: Matt Roper
> Cc: Bob Paauwe
> Cc: Ville Syrjälä
> Reviewed-by: Hans de Goede
> Signed-off-by: Vivek Kasireddy
Since v3 is identical to v2 except for the commit message, I think we
can apply this based on v2's CI results. The only CI failure was an
incomplete on a non-DSI TGL system and the incomplete happened long
after the VBT parsing changes here would have had any effect.
Applied to dinq. Thanks for the patch and review.
Matt
> ---
> drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 99 +++-
> 2 files changed, 100 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
> b/drivers/gpu/drm/i915/display/intel_dsi.h
> index 7481a5aa3084..6cef1356b4e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
> @@ -69,6 +69,9 @@ struct intel_dsi {
> /* number of DSI lanes */
> unsigned int lane_count;
>
> + /* i2c bus associated with the slave device */
> + int i2c_bus_num;
> +
> /*
>* video mode pixel format
>*
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index 0032161e0f76..89fb0d90b694 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -86,6 +86,12 @@ static struct gpio_map vlv_gpio_table[] = {
> { VLV_GPIO_NC_11_PANEL1_BKLTCTL },
> };
>
> +struct i2c_adapter_lookup {
> + u16 slave_addr;
> + struct intel_dsi *intel_dsi;
> + acpi_handle dev_handle;
> +};
> +
> #define CHV_GPIO_IDX_START_N 0
> #define CHV_GPIO_IDX_START_E 73
> #define CHV_GPIO_IDX_START_SW100
> @@ -378,11 +384,98 @@ static const u8 *mipi_exec_gpio(struct intel_dsi
> *intel_dsi, const u8 *data)
> return data;
> }
>
> +static int i2c_adapter_lookup(struct acpi_resource *ares, void *data)
> +{
> + struct i2c_adapter_lookup *lookup = data;
> + struct intel_dsi *intel_dsi = lookup->intel_dsi;
> + struct acpi_resource_i2c_serialbus *sb;
> + struct i2c_adapter *adapter;
> + acpi_handle adapter_handle;
> + acpi_status status;
> +
> + if (intel_dsi->i2c_bus_num >= 0 ||
> + !i2c_acpi_get_i2c_resource(ares, &sb))
> + return 1;
> +
> + if (lookup->slave_addr != sb->slave_address)
> + return 1;
> +
> + status = acpi_get_handle(lookup->dev_handle,
> + sb->resource_source.string_ptr,
> + &adapter_handle);
> + if (ACPI_FAILURE(status))
> + return 1;
> +
> + adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
> + if (adapter)
> + intel_dsi->i2c_bus_num = adapter->nr;
> +
> + return 1;
> +}
> +
> static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
> {
> - DRM_DEBUG_KMS("Skipping I2C element execution\n");
> + struct drm_device *drm_dev = intel_dsi->base.base.dev;
> + struct device *dev = &drm_dev->pdev->dev;
> + struct i2c_adapter *adapter;
> + struct acpi_device *acpi_dev;
> + struct list_head resource_list;
> + struct i2c_adapter_lookup lookup;
> + struct i2c_msg msg;
> + int ret;
> + u8 vbt_i2c_bus_num = *(data + 2);
> + u16 slave_addr = *(u16 *)(data + 3);
> + u8 reg_offset = *(data + 5);
> + u8 payload_size = *(data + 6);
> + u8 *payload_data;
> +
> + if (intel_dsi->i2c_bus_num < 0) {
> + intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
> +
> + acpi_dev = ACPI_COMPANION(dev);
> + if (acpi_dev) {
> + memset(&lookup, 0, sizeof(lookup));
> + lookup.slave_addr = slave_addr;
> + lookup.intel_dsi = intel_dsi;
> + lookup.dev_handle
[Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block (v3)
Parsing the i2c element is mainly done to transfer the payload from the
MIPI sequence block to the relevant slave device. In some cases, the
commands that are part of the payload can be used to turn on the backlight.
This patch is actually a refactored version of this old patch:
https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
In addition to the refactoring, the original patch is augmented by looking up
the i2c bus from ACPI NS instead of relying on the bus number provided
in the VBT.
This patch was tested on Aava Mobile's Inari 10 tablet. It enabled
turning on the backlight by transfering the payload to the device.
v2:
- Add DRM_DEV_ERROR for invalid adapter and failed transfer and also
drop the DRM_DEBUG that existed originally. (Hans)
- Add two gotos instead of one to clean things up properly.
v3:
- Identify the device on which this patch was tested in the commit
message (Ville)
Cc: Hans de Goede
Cc: Nabendu Maiti
Cc: Matt Roper
Cc: Bob Paauwe
Cc: Ville Syrjälä
Reviewed-by: Hans de Goede
Signed-off-by: Vivek Kasireddy
---
drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 99 +++-
2 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
b/drivers/gpu/drm/i915/display/intel_dsi.h
index 7481a5aa3084..6cef1356b4e6 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.h
+++ b/drivers/gpu/drm/i915/display/intel_dsi.h
@@ -69,6 +69,9 @@ struct intel_dsi {
/* number of DSI lanes */
unsigned int lane_count;
+ /* i2c bus associated with the slave device */
+ int i2c_bus_num;
+
/*
* video mode pixel format
*
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
index 0032161e0f76..89fb0d90b694 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
@@ -86,6 +86,12 @@ static struct gpio_map vlv_gpio_table[] = {
{ VLV_GPIO_NC_11_PANEL1_BKLTCTL },
};
+struct i2c_adapter_lookup {
+ u16 slave_addr;
+ struct intel_dsi *intel_dsi;
+ acpi_handle dev_handle;
+};
+
#define CHV_GPIO_IDX_START_N 0
#define CHV_GPIO_IDX_START_E 73
#define CHV_GPIO_IDX_START_SW 100
@@ -378,11 +384,98 @@ static const u8 *mipi_exec_gpio(struct intel_dsi
*intel_dsi, const u8 *data)
return data;
}
+static int i2c_adapter_lookup(struct acpi_resource *ares, void *data)
+{
+ struct i2c_adapter_lookup *lookup = data;
+ struct intel_dsi *intel_dsi = lookup->intel_dsi;
+ struct acpi_resource_i2c_serialbus *sb;
+ struct i2c_adapter *adapter;
+ acpi_handle adapter_handle;
+ acpi_status status;
+
+ if (intel_dsi->i2c_bus_num >= 0 ||
+ !i2c_acpi_get_i2c_resource(ares, &sb))
+ return 1;
+
+ if (lookup->slave_addr != sb->slave_address)
+ return 1;
+
+ status = acpi_get_handle(lookup->dev_handle,
+sb->resource_source.string_ptr,
+&adapter_handle);
+ if (ACPI_FAILURE(status))
+ return 1;
+
+ adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
+ if (adapter)
+ intel_dsi->i2c_bus_num = adapter->nr;
+
+ return 1;
+}
+
static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
{
- DRM_DEBUG_KMS("Skipping I2C element execution\n");
+ struct drm_device *drm_dev = intel_dsi->base.base.dev;
+ struct device *dev = &drm_dev->pdev->dev;
+ struct i2c_adapter *adapter;
+ struct acpi_device *acpi_dev;
+ struct list_head resource_list;
+ struct i2c_adapter_lookup lookup;
+ struct i2c_msg msg;
+ int ret;
+ u8 vbt_i2c_bus_num = *(data + 2);
+ u16 slave_addr = *(u16 *)(data + 3);
+ u8 reg_offset = *(data + 5);
+ u8 payload_size = *(data + 6);
+ u8 *payload_data;
+
+ if (intel_dsi->i2c_bus_num < 0) {
+ intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
+
+ acpi_dev = ACPI_COMPANION(dev);
+ if (acpi_dev) {
+ memset(&lookup, 0, sizeof(lookup));
+ lookup.slave_addr = slave_addr;
+ lookup.intel_dsi = intel_dsi;
+ lookup.dev_handle = acpi_device_handle(acpi_dev);
+
+ INIT_LIST_HEAD(&resource_list);
+ acpi_dev_get_resources(acpi_dev, &resource_list,
+ i2c_adapter_lookup,
+ &lookup);
+ acpi_dev_free_resource_list(&resource_list);
+ }
+ }
- return data + *(data + 6) + 7;
+ adapter = i2c_get_adapter(intel_dsi->i2c_bus_num);
+ if (!adapter) {
+ DRM_DEV_ERROR(dev, "C
Re: [Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block
On Tue, Jan 07, 2020 at 06:49:04PM +0200, Ville Syrjälä wrote:
> On Fri, Jan 03, 2020 at 04:00:54PM -0800, Vivek Kasireddy wrote:
> > On Fri, 3 Jan 2020 12:05:11 +0100
> > Hans de Goede wrote:
> > Hi Hans,
> >
> > > Hi Vivek,
> > >
> > > On 03-01-2020 01:00, Vivek Kasireddy wrote:
> > > > Parsing the i2c element is mainly done to transfer the payload from
> > > > the MIPI sequence block to the relevant slave device. In some
> > > > cases, the commands that are part of the payload can be used to
> > > > turn on the backlight.
> > > >
> > > > This patch is actually a refactored version of this old patch:
> > > > https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
> > > >
> > > > In addition to the refactoring, the old patch is augmented by
> > > > looking up the i2c bus from ACPI NS instead of relying on the bus
> > > > number provided in the VBT.
> > > >
> > > > Cc: Deepak M
> > > > Cc: Nabendu Maiti
> > > > Cc: Matt Roper
> > > > Cc: Bob Paauwe
> > > > Signed-off-by: Vivek Kasireddy
> > >
> > > Thank you for this patch, I have been doing a lot of work to make
> > > DSI panels on Bay Trail and Cherry Trail devices work better, as such
> > > I've done a lot of testing of DSI panels. But I have never seen any
> > > MIPI sequences actually use the i2c commands. May I ask how you have
> > > tested this? Do you have a device which actually uses the i2c
> > > commands?
> > Oh, they sure exist; we do have a device that uses i2c commands to turn
> > on the backlight that we have tested this patch on.
>
> And what exactly is that device? That is valuable information that the
> commit message should contain.
I'm not sure if we're allowed to disclose that information. I believe
Vivek is working with an engineering sample and the device itself might
not have been publicly announced by the device manufacturer yet.
Matt
>
> >
> > >
> > > I also have some small review comments inline:
> > >
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
> > > > drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 93
> > > > 2 files changed, 96 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
> > > > b/drivers/gpu/drm/i915/display/intel_dsi.h index
> > > > b15be5814599..5651bc8aa5c2 100644 ---
> > > > a/drivers/gpu/drm/i915/display/intel_dsi.h +++
> > > > b/drivers/gpu/drm/i915/display/intel_dsi.h @@ -68,6 +68,9 @@ struct
> > > > intel_dsi { /* number of DSI lanes */
> > > > unsigned int lane_count;
> > > >
> > > > + /* i2c bus associated with the slave device */
> > > > + int i2c_bus_num;
> > > > +
> > > > /*
> > > > * video mode pixel format
> > > > *
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> > > > b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c index
> > > > f90946c912ee..60441a5a3dba 100644 ---
> > > > a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c +++
> > > > b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c @@ -83,6 +83,12 @@
> > > > static struct gpio_map vlv_gpio_table[] = { {
> > > > VLV_GPIO_NC_11_PANEL1_BKLTCTL }, };
> > > >
> > > > +struct i2c_adapter_lookup {
> > > > + u16 slave_addr;
> > > > + struct intel_dsi *intel_dsi;
> > > > + acpi_handle dev_handle;
> > > > +};
> > > > +
> > > > #define CHV_GPIO_IDX_START_N 0
> > > > #define CHV_GPIO_IDX_START_E 73
> > > > #define CHV_GPIO_IDX_START_SW 100
> > > > @@ -375,8 +381,93 @@ static const u8 *mipi_exec_gpio(struct
> > > > intel_dsi *intel_dsi, const u8 *data) return data;
> > > > }
> > > >
> > > > +static int i2c_adapter_lookup(struct acpi_resource *ares, void
> > > > *data) +{
> > > > + struct i2c_adapter_lookup *lookup = data;
> > > > + struct intel_dsi *intel_dsi = lookup->intel_dsi;
> > > > + struct acpi_resource_i2c_serialbus *sb;
> > > > + struct i2c_adapter *adapter;
> > > > + acpi_handle adapter_handle;
> > > > + acpi_status status;
> > > > +
> > > > + if (intel_dsi->i2c_bus_num >= 0 ||
> > > > + !i2c_acpi_get_i2c_resource(ares, &sb))
> > > > + return 1;
> > > > +
> > > > + if (lookup->slave_addr != sb->slave_address)
> > > > + return 1;
> > > > +
> > > > + status = acpi_get_handle(lookup->dev_handle,
> > > > +sb->resource_source.string_ptr,
> > > > +&adapter_handle);
> > > > + if (ACPI_FAILURE(status))
> > > > + return 1;
> > > > +
> > > > + adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
> > > > + if (adapter)
> > > > + intel_dsi->i2c_bus_num = adapter->nr;
> > > > +
> > > > + return 1;
> > > > +}
> > > > +
> > > > static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const
> > > > u8 *data) {
> > > > + struct drm_device *dev = intel_dsi->base.base.dev;
> > > > + struct i2c_adapter
Re: [Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block
On Fri, Jan 03, 2020 at 04:00:54PM -0800, Vivek Kasireddy wrote:
> On Fri, 3 Jan 2020 12:05:11 +0100
> Hans de Goede wrote:
> Hi Hans,
>
> > Hi Vivek,
> >
> > On 03-01-2020 01:00, Vivek Kasireddy wrote:
> > > Parsing the i2c element is mainly done to transfer the payload from
> > > the MIPI sequence block to the relevant slave device. In some
> > > cases, the commands that are part of the payload can be used to
> > > turn on the backlight.
> > >
> > > This patch is actually a refactored version of this old patch:
> > > https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
> > >
> > > In addition to the refactoring, the old patch is augmented by
> > > looking up the i2c bus from ACPI NS instead of relying on the bus
> > > number provided in the VBT.
> > >
> > > Cc: Deepak M
> > > Cc: Nabendu Maiti
> > > Cc: Matt Roper
> > > Cc: Bob Paauwe
> > > Signed-off-by: Vivek Kasireddy
> >
> > Thank you for this patch, I have been doing a lot of work to make
> > DSI panels on Bay Trail and Cherry Trail devices work better, as such
> > I've done a lot of testing of DSI panels. But I have never seen any
> > MIPI sequences actually use the i2c commands. May I ask how you have
> > tested this? Do you have a device which actually uses the i2c
> > commands?
> Oh, they sure exist; we do have a device that uses i2c commands to turn
> on the backlight that we have tested this patch on.
And what exactly is that device? That is valuable information that the
commit message should contain.
>
> >
> > I also have some small review comments inline:
> >
> > > ---
> > > drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
> > > drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 93
> > > 2 files changed, 96 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
> > > b/drivers/gpu/drm/i915/display/intel_dsi.h index
> > > b15be5814599..5651bc8aa5c2 100644 ---
> > > a/drivers/gpu/drm/i915/display/intel_dsi.h +++
> > > b/drivers/gpu/drm/i915/display/intel_dsi.h @@ -68,6 +68,9 @@ struct
> > > intel_dsi { /* number of DSI lanes */
> > > unsigned int lane_count;
> > >
> > > + /* i2c bus associated with the slave device */
> > > + int i2c_bus_num;
> > > +
> > > /*
> > >* video mode pixel format
> > >*
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> > > b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c index
> > > f90946c912ee..60441a5a3dba 100644 ---
> > > a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c +++
> > > b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c @@ -83,6 +83,12 @@
> > > static struct gpio_map vlv_gpio_table[] = { {
> > > VLV_GPIO_NC_11_PANEL1_BKLTCTL }, };
> > >
> > > +struct i2c_adapter_lookup {
> > > + u16 slave_addr;
> > > + struct intel_dsi *intel_dsi;
> > > + acpi_handle dev_handle;
> > > +};
> > > +
> > > #define CHV_GPIO_IDX_START_N0
> > > #define CHV_GPIO_IDX_START_E73
> > > #define CHV_GPIO_IDX_START_SW 100
> > > @@ -375,8 +381,93 @@ static const u8 *mipi_exec_gpio(struct
> > > intel_dsi *intel_dsi, const u8 *data) return data;
> > > }
> > >
> > > +static int i2c_adapter_lookup(struct acpi_resource *ares, void
> > > *data) +{
> > > + struct i2c_adapter_lookup *lookup = data;
> > > + struct intel_dsi *intel_dsi = lookup->intel_dsi;
> > > + struct acpi_resource_i2c_serialbus *sb;
> > > + struct i2c_adapter *adapter;
> > > + acpi_handle adapter_handle;
> > > + acpi_status status;
> > > +
> > > + if (intel_dsi->i2c_bus_num >= 0 ||
> > > + !i2c_acpi_get_i2c_resource(ares, &sb))
> > > + return 1;
> > > +
> > > + if (lookup->slave_addr != sb->slave_address)
> > > + return 1;
> > > +
> > > + status = acpi_get_handle(lookup->dev_handle,
> > > + sb->resource_source.string_ptr,
> > > + &adapter_handle);
> > > + if (ACPI_FAILURE(status))
> > > + return 1;
> > > +
> > > + adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
> > > + if (adapter)
> > > + intel_dsi->i2c_bus_num = adapter->nr;
> > > +
> > > + return 1;
> > > +}
> > > +
> > > static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const
> > > u8 *data) {
> > > + struct drm_device *dev = intel_dsi->base.base.dev;
> > > + struct i2c_adapter *adapter;
> > > + struct acpi_device *acpi_dev;
> > > + struct list_head resource_list;
> > > + struct i2c_adapter_lookup lookup;
> > > + struct i2c_msg msg;
> > > + int ret;
> > > + u8 vbt_i2c_bus_num = *(data + 2);
> > > + u16 slave_addr = *(u16 *)(data + 3);
> > > + u8 reg_offset = *(data + 5);
> > > + u8 payload_size = *(data + 6);
> > > + u8 *payload_data;
> > > +
> > > + if (intel_dsi->i2c_bus_num < 0) {
> > > + intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
> > > +
> > > + acpi_dev = ACPI_COMPANION(&dev->pdev->dev);
> > > + if (acpi_dev) {
> > > + memset(&lookup, 0, sizeof(lookup));
> > > + l
Re: [Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block (v2)
Hi,
On 04-01-2020 02:09, Vivek Kasireddy wrote:
Parsing the i2c element is mainly done to transfer the payload from the
MIPI sequence block to the relevant slave device. In some cases, the
commands that are part of the payload can be used to turn on the backlight.
This patch is actually a refactored version of this old patch:
https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
In addition to the refactoring, the original patch is augmented by looking up
the i2c bus from ACPI NS instead of relying on the bus number provided
in the VBT.
v2:
- Add DRM_DEV_ERROR for invalid adapter and failed transfer and also
drop the DRM_DEBUG that existed originally. (Hans)
- Add two gotos instead of one to clean things up properly.
CC: Hans de Goede
Cc: Nabendu Maiti
Cc: Matt Roper
Cc: Bob Paauwe
Signed-off-by: Vivek Kasireddy
v2 looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
---
drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 99 +++-
2 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
b/drivers/gpu/drm/i915/display/intel_dsi.h
index b15be5814599..5651bc8aa5c2 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.h
+++ b/drivers/gpu/drm/i915/display/intel_dsi.h
@@ -68,6 +68,9 @@ struct intel_dsi {
/* number of DSI lanes */
unsigned int lane_count;
+ /* i2c bus associated with the slave device */
+ int i2c_bus_num;
+
/*
* video mode pixel format
*
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
index f90946c912ee..35fcef7c0d70 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
@@ -83,6 +83,12 @@ static struct gpio_map vlv_gpio_table[] = {
{ VLV_GPIO_NC_11_PANEL1_BKLTCTL },
};
+struct i2c_adapter_lookup {
+ u16 slave_addr;
+ struct intel_dsi *intel_dsi;
+ acpi_handle dev_handle;
+};
+
#define CHV_GPIO_IDX_START_N 0
#define CHV_GPIO_IDX_START_E 73
#define CHV_GPIO_IDX_START_SW 100
@@ -375,11 +381,98 @@ static const u8 *mipi_exec_gpio(struct intel_dsi
*intel_dsi, const u8 *data)
return data;
}
+static int i2c_adapter_lookup(struct acpi_resource *ares, void *data)
+{
+ struct i2c_adapter_lookup *lookup = data;
+ struct intel_dsi *intel_dsi = lookup->intel_dsi;
+ struct acpi_resource_i2c_serialbus *sb;
+ struct i2c_adapter *adapter;
+ acpi_handle adapter_handle;
+ acpi_status status;
+
+ if (intel_dsi->i2c_bus_num >= 0 ||
+ !i2c_acpi_get_i2c_resource(ares, &sb))
+ return 1;
+
+ if (lookup->slave_addr != sb->slave_address)
+ return 1;
+
+ status = acpi_get_handle(lookup->dev_handle,
+sb->resource_source.string_ptr,
+&adapter_handle);
+ if (ACPI_FAILURE(status))
+ return 1;
+
+ adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
+ if (adapter)
+ intel_dsi->i2c_bus_num = adapter->nr;
+
+ return 1;
+}
+
static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
{
- DRM_DEBUG_KMS("Skipping I2C element execution\n");
+ struct drm_device *drm_dev = intel_dsi->base.base.dev;
+ struct device *dev = &drm_dev->pdev->dev;
+ struct i2c_adapter *adapter;
+ struct acpi_device *acpi_dev;
+ struct list_head resource_list;
+ struct i2c_adapter_lookup lookup;
+ struct i2c_msg msg;
+ int ret;
+ u8 vbt_i2c_bus_num = *(data + 2);
+ u16 slave_addr = *(u16 *)(data + 3);
+ u8 reg_offset = *(data + 5);
+ u8 payload_size = *(data + 6);
+ u8 *payload_data;
+
+ if (intel_dsi->i2c_bus_num < 0) {
+ intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
+
+ acpi_dev = ACPI_COMPANION(dev);
+ if (acpi_dev) {
+ memset(&lookup, 0, sizeof(lookup));
+ lookup.slave_addr = slave_addr;
+ lookup.intel_dsi = intel_dsi;
+ lookup.dev_handle = acpi_device_handle(acpi_dev);
+
+ INIT_LIST_HEAD(&resource_list);
+ acpi_dev_get_resources(acpi_dev, &resource_list,
+ i2c_adapter_lookup,
+ &lookup);
+ acpi_dev_free_resource_list(&resource_list);
+ }
+ }
- return data + *(data + 6) + 7;
+ adapter = i2c_get_adapter(intel_dsi->i2c_bus_num);
+ if (!adapter) {
+ DRM_DEV_ERROR(dev, "Cannot find a valid i2c bus for xfer\n");
+ goto err_bus;
+ }
+
+ payload_data = kzalloc(payload_size + 1, GFP_KERNEL);
Re: [Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block
Hi,
On 04-01-2020 01:00, Vivek Kasireddy wrote:
On Fri, 3 Jan 2020 12:05:11 +0100
Hans de Goede wrote:
Hi Hans,
Hi Vivek,
On 03-01-2020 01:00, Vivek Kasireddy wrote:
Parsing the i2c element is mainly done to transfer the payload from
the MIPI sequence block to the relevant slave device. In some
cases, the commands that are part of the payload can be used to
turn on the backlight.
This patch is actually a refactored version of this old patch:
https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
In addition to the refactoring, the old patch is augmented by
looking up the i2c bus from ACPI NS instead of relying on the bus
number provided in the VBT.
Cc: Deepak M
Cc: Nabendu Maiti
Cc: Matt Roper
Cc: Bob Paauwe
Signed-off-by: Vivek Kasireddy
Thank you for this patch, I have been doing a lot of work to make
DSI panels on Bay Trail and Cherry Trail devices work better, as such
I've done a lot of testing of DSI panels. But I have never seen any
MIPI sequences actually use the i2c commands. May I ask how you have
tested this? Do you have a device which actually uses the i2c
commands?
Oh, they sure exist; we do have a device that uses i2c commands to turn
on the backlight that we have tested this patch on.
Ah, ok, good. I was a bit worried this patch was not tested with
any devices actually using the i2c stuff. So I'm happy to hear that it
has been tested.
Regards,
Hans
I also have some small review comments inline:
---
drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 93
2 files changed, 96 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
b/drivers/gpu/drm/i915/display/intel_dsi.h index
b15be5814599..5651bc8aa5c2 100644 ---
a/drivers/gpu/drm/i915/display/intel_dsi.h +++
b/drivers/gpu/drm/i915/display/intel_dsi.h @@ -68,6 +68,9 @@ struct
intel_dsi { /* number of DSI lanes */
unsigned int lane_count;
+ /* i2c bus associated with the slave device */
+ int i2c_bus_num;
+
/*
* video mode pixel format
*
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c index
f90946c912ee..60441a5a3dba 100644 ---
a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c +++
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c @@ -83,6 +83,12 @@
static struct gpio_map vlv_gpio_table[] = { {
VLV_GPIO_NC_11_PANEL1_BKLTCTL }, };
+struct i2c_adapter_lookup {
+ u16 slave_addr;
+ struct intel_dsi *intel_dsi;
+ acpi_handle dev_handle;
+};
+
#define CHV_GPIO_IDX_START_N 0
#define CHV_GPIO_IDX_START_E 73
#define CHV_GPIO_IDX_START_SW100
@@ -375,8 +381,93 @@ static const u8 *mipi_exec_gpio(struct
intel_dsi *intel_dsi, const u8 *data) return data;
}
+static int i2c_adapter_lookup(struct acpi_resource *ares, void
*data) +{
+ struct i2c_adapter_lookup *lookup = data;
+ struct intel_dsi *intel_dsi = lookup->intel_dsi;
+ struct acpi_resource_i2c_serialbus *sb;
+ struct i2c_adapter *adapter;
+ acpi_handle adapter_handle;
+ acpi_status status;
+
+ if (intel_dsi->i2c_bus_num >= 0 ||
+ !i2c_acpi_get_i2c_resource(ares, &sb))
+ return 1;
+
+ if (lookup->slave_addr != sb->slave_address)
+ return 1;
+
+ status = acpi_get_handle(lookup->dev_handle,
+sb->resource_source.string_ptr,
+&adapter_handle);
+ if (ACPI_FAILURE(status))
+ return 1;
+
+ adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
+ if (adapter)
+ intel_dsi->i2c_bus_num = adapter->nr;
+
+ return 1;
+}
+
static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const
u8 *data) {
+ struct drm_device *dev = intel_dsi->base.base.dev;
+ struct i2c_adapter *adapter;
+ struct acpi_device *acpi_dev;
+ struct list_head resource_list;
+ struct i2c_adapter_lookup lookup;
+ struct i2c_msg msg;
+ int ret;
+ u8 vbt_i2c_bus_num = *(data + 2);
+ u16 slave_addr = *(u16 *)(data + 3);
+ u8 reg_offset = *(data + 5);
+ u8 payload_size = *(data + 6);
+ u8 *payload_data;
+
+ if (intel_dsi->i2c_bus_num < 0) {
+ intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
+
+ acpi_dev = ACPI_COMPANION(&dev->pdev->dev);
+ if (acpi_dev) {
+ memset(&lookup, 0, sizeof(lookup));
+ lookup.slave_addr = slave_addr;
+ lookup.intel_dsi = intel_dsi;
+ lookup.dev_handle =
acpi_device_handle(acpi_dev); +
+ INIT_LIST_HEAD(&resource_list);
+ acpi_dev_get_resources(acpi_dev,
&resource_list,
+ i2c_adapter_lookup,
+
[Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block (v2)
Parsing the i2c element is mainly done to transfer the payload from the
MIPI sequence block to the relevant slave device. In some cases, the
commands that are part of the payload can be used to turn on the backlight.
This patch is actually a refactored version of this old patch:
https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
In addition to the refactoring, the original patch is augmented by looking up
the i2c bus from ACPI NS instead of relying on the bus number provided
in the VBT.
v2:
- Add DRM_DEV_ERROR for invalid adapter and failed transfer and also
drop the DRM_DEBUG that existed originally. (Hans)
- Add two gotos instead of one to clean things up properly.
CC: Hans de Goede
Cc: Nabendu Maiti
Cc: Matt Roper
Cc: Bob Paauwe
Signed-off-by: Vivek Kasireddy
---
drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 99 +++-
2 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
b/drivers/gpu/drm/i915/display/intel_dsi.h
index b15be5814599..5651bc8aa5c2 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.h
+++ b/drivers/gpu/drm/i915/display/intel_dsi.h
@@ -68,6 +68,9 @@ struct intel_dsi {
/* number of DSI lanes */
unsigned int lane_count;
+ /* i2c bus associated with the slave device */
+ int i2c_bus_num;
+
/*
* video mode pixel format
*
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
index f90946c912ee..35fcef7c0d70 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
@@ -83,6 +83,12 @@ static struct gpio_map vlv_gpio_table[] = {
{ VLV_GPIO_NC_11_PANEL1_BKLTCTL },
};
+struct i2c_adapter_lookup {
+ u16 slave_addr;
+ struct intel_dsi *intel_dsi;
+ acpi_handle dev_handle;
+};
+
#define CHV_GPIO_IDX_START_N 0
#define CHV_GPIO_IDX_START_E 73
#define CHV_GPIO_IDX_START_SW 100
@@ -375,11 +381,98 @@ static const u8 *mipi_exec_gpio(struct intel_dsi
*intel_dsi, const u8 *data)
return data;
}
+static int i2c_adapter_lookup(struct acpi_resource *ares, void *data)
+{
+ struct i2c_adapter_lookup *lookup = data;
+ struct intel_dsi *intel_dsi = lookup->intel_dsi;
+ struct acpi_resource_i2c_serialbus *sb;
+ struct i2c_adapter *adapter;
+ acpi_handle adapter_handle;
+ acpi_status status;
+
+ if (intel_dsi->i2c_bus_num >= 0 ||
+ !i2c_acpi_get_i2c_resource(ares, &sb))
+ return 1;
+
+ if (lookup->slave_addr != sb->slave_address)
+ return 1;
+
+ status = acpi_get_handle(lookup->dev_handle,
+sb->resource_source.string_ptr,
+&adapter_handle);
+ if (ACPI_FAILURE(status))
+ return 1;
+
+ adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
+ if (adapter)
+ intel_dsi->i2c_bus_num = adapter->nr;
+
+ return 1;
+}
+
static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
{
- DRM_DEBUG_KMS("Skipping I2C element execution\n");
+ struct drm_device *drm_dev = intel_dsi->base.base.dev;
+ struct device *dev = &drm_dev->pdev->dev;
+ struct i2c_adapter *adapter;
+ struct acpi_device *acpi_dev;
+ struct list_head resource_list;
+ struct i2c_adapter_lookup lookup;
+ struct i2c_msg msg;
+ int ret;
+ u8 vbt_i2c_bus_num = *(data + 2);
+ u16 slave_addr = *(u16 *)(data + 3);
+ u8 reg_offset = *(data + 5);
+ u8 payload_size = *(data + 6);
+ u8 *payload_data;
+
+ if (intel_dsi->i2c_bus_num < 0) {
+ intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
+
+ acpi_dev = ACPI_COMPANION(dev);
+ if (acpi_dev) {
+ memset(&lookup, 0, sizeof(lookup));
+ lookup.slave_addr = slave_addr;
+ lookup.intel_dsi = intel_dsi;
+ lookup.dev_handle = acpi_device_handle(acpi_dev);
+
+ INIT_LIST_HEAD(&resource_list);
+ acpi_dev_get_resources(acpi_dev, &resource_list,
+ i2c_adapter_lookup,
+ &lookup);
+ acpi_dev_free_resource_list(&resource_list);
+ }
+ }
- return data + *(data + 6) + 7;
+ adapter = i2c_get_adapter(intel_dsi->i2c_bus_num);
+ if (!adapter) {
+ DRM_DEV_ERROR(dev, "Cannot find a valid i2c bus for xfer\n");
+ goto err_bus;
+ }
+
+ payload_data = kzalloc(payload_size + 1, GFP_KERNEL);
+ if (!payload_data)
+ goto err_alloc;
+
+ payload_data[0] = reg_offset;
+ memcpy(&payload_data
Re: [Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block
On Fri, 3 Jan 2020 12:05:11 +0100
Hans de Goede wrote:
Hi Hans,
> Hi Vivek,
>
> On 03-01-2020 01:00, Vivek Kasireddy wrote:
> > Parsing the i2c element is mainly done to transfer the payload from
> > the MIPI sequence block to the relevant slave device. In some
> > cases, the commands that are part of the payload can be used to
> > turn on the backlight.
> >
> > This patch is actually a refactored version of this old patch:
> > https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
> >
> > In addition to the refactoring, the old patch is augmented by
> > looking up the i2c bus from ACPI NS instead of relying on the bus
> > number provided in the VBT.
> >
> > Cc: Deepak M
> > Cc: Nabendu Maiti
> > Cc: Matt Roper
> > Cc: Bob Paauwe
> > Signed-off-by: Vivek Kasireddy
>
> Thank you for this patch, I have been doing a lot of work to make
> DSI panels on Bay Trail and Cherry Trail devices work better, as such
> I've done a lot of testing of DSI panels. But I have never seen any
> MIPI sequences actually use the i2c commands. May I ask how you have
> tested this? Do you have a device which actually uses the i2c
> commands?
Oh, they sure exist; we do have a device that uses i2c commands to turn
on the backlight that we have tested this patch on.
>
> I also have some small review comments inline:
>
> > ---
> > drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
> > drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 93
> > 2 files changed, 96 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
> > b/drivers/gpu/drm/i915/display/intel_dsi.h index
> > b15be5814599..5651bc8aa5c2 100644 ---
> > a/drivers/gpu/drm/i915/display/intel_dsi.h +++
> > b/drivers/gpu/drm/i915/display/intel_dsi.h @@ -68,6 +68,9 @@ struct
> > intel_dsi { /* number of DSI lanes */
> > unsigned int lane_count;
> >
> > + /* i2c bus associated with the slave device */
> > + int i2c_bus_num;
> > +
> > /*
> > * video mode pixel format
> > *
> > diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> > b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c index
> > f90946c912ee..60441a5a3dba 100644 ---
> > a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c +++
> > b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c @@ -83,6 +83,12 @@
> > static struct gpio_map vlv_gpio_table[] = { {
> > VLV_GPIO_NC_11_PANEL1_BKLTCTL }, };
> >
> > +struct i2c_adapter_lookup {
> > + u16 slave_addr;
> > + struct intel_dsi *intel_dsi;
> > + acpi_handle dev_handle;
> > +};
> > +
> > #define CHV_GPIO_IDX_START_N 0
> > #define CHV_GPIO_IDX_START_E 73
> > #define CHV_GPIO_IDX_START_SW 100
> > @@ -375,8 +381,93 @@ static const u8 *mipi_exec_gpio(struct
> > intel_dsi *intel_dsi, const u8 *data) return data;
> > }
> >
> > +static int i2c_adapter_lookup(struct acpi_resource *ares, void
> > *data) +{
> > + struct i2c_adapter_lookup *lookup = data;
> > + struct intel_dsi *intel_dsi = lookup->intel_dsi;
> > + struct acpi_resource_i2c_serialbus *sb;
> > + struct i2c_adapter *adapter;
> > + acpi_handle adapter_handle;
> > + acpi_status status;
> > +
> > + if (intel_dsi->i2c_bus_num >= 0 ||
> > + !i2c_acpi_get_i2c_resource(ares, &sb))
> > + return 1;
> > +
> > + if (lookup->slave_addr != sb->slave_address)
> > + return 1;
> > +
> > + status = acpi_get_handle(lookup->dev_handle,
> > +sb->resource_source.string_ptr,
> > +&adapter_handle);
> > + if (ACPI_FAILURE(status))
> > + return 1;
> > +
> > + adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
> > + if (adapter)
> > + intel_dsi->i2c_bus_num = adapter->nr;
> > +
> > + return 1;
> > +}
> > +
> > static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const
> > u8 *data) {
> > + struct drm_device *dev = intel_dsi->base.base.dev;
> > + struct i2c_adapter *adapter;
> > + struct acpi_device *acpi_dev;
> > + struct list_head resource_list;
> > + struct i2c_adapter_lookup lookup;
> > + struct i2c_msg msg;
> > + int ret;
> > + u8 vbt_i2c_bus_num = *(data + 2);
> > + u16 slave_addr = *(u16 *)(data + 3);
> > + u8 reg_offset = *(data + 5);
> > + u8 payload_size = *(data + 6);
> > + u8 *payload_data;
> > +
> > + if (intel_dsi->i2c_bus_num < 0) {
> > + intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
> > +
> > + acpi_dev = ACPI_COMPANION(&dev->pdev->dev);
> > + if (acpi_dev) {
> > + memset(&lookup, 0, sizeof(lookup));
> > + lookup.slave_addr = slave_addr;
> > + lookup.intel_dsi = intel_dsi;
> > + lookup.dev_handle =
> > acpi_device_handle(acpi_dev); +
> > + INIT_LIST_HEAD(&resource_list);
> > + acpi_dev_get_resources(acpi_dev,
> > &resource_list,
> > + i2c_adapter_
Re: [Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block
Hi Vivek,
On 03-01-2020 01:00, Vivek Kasireddy wrote:
Parsing the i2c element is mainly done to transfer the payload from the
MIPI sequence block to the relevant slave device. In some cases, the
commands that are part of the payload can be used to turn on the backlight.
This patch is actually a refactored version of this old patch:
https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
In addition to the refactoring, the old patch is augmented by looking up
the i2c bus from ACPI NS instead of relying on the bus number provided
in the VBT.
Cc: Deepak M
Cc: Nabendu Maiti
Cc: Matt Roper
Cc: Bob Paauwe
Signed-off-by: Vivek Kasireddy
Thank you for this patch, I have been doing a lot of work to make
DSI panels on Bay Trail and Cherry Trail devices work better, as such
I've done a lot of testing of DSI panels. But I have never seen any
MIPI sequences actually use the i2c commands. May I ask how you have
tested this? Do you have a device which actually uses the i2c commands?
I also have some small review comments inline:
---
drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 93
2 files changed, 96 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
b/drivers/gpu/drm/i915/display/intel_dsi.h
index b15be5814599..5651bc8aa5c2 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.h
+++ b/drivers/gpu/drm/i915/display/intel_dsi.h
@@ -68,6 +68,9 @@ struct intel_dsi {
/* number of DSI lanes */
unsigned int lane_count;
+ /* i2c bus associated with the slave device */
+ int i2c_bus_num;
+
/*
* video mode pixel format
*
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
index f90946c912ee..60441a5a3dba 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
@@ -83,6 +83,12 @@ static struct gpio_map vlv_gpio_table[] = {
{ VLV_GPIO_NC_11_PANEL1_BKLTCTL },
};
+struct i2c_adapter_lookup {
+ u16 slave_addr;
+ struct intel_dsi *intel_dsi;
+ acpi_handle dev_handle;
+};
+
#define CHV_GPIO_IDX_START_N 0
#define CHV_GPIO_IDX_START_E 73
#define CHV_GPIO_IDX_START_SW 100
@@ -375,8 +381,93 @@ static const u8 *mipi_exec_gpio(struct intel_dsi
*intel_dsi, const u8 *data)
return data;
}
+static int i2c_adapter_lookup(struct acpi_resource *ares, void *data)
+{
+ struct i2c_adapter_lookup *lookup = data;
+ struct intel_dsi *intel_dsi = lookup->intel_dsi;
+ struct acpi_resource_i2c_serialbus *sb;
+ struct i2c_adapter *adapter;
+ acpi_handle adapter_handle;
+ acpi_status status;
+
+ if (intel_dsi->i2c_bus_num >= 0 ||
+ !i2c_acpi_get_i2c_resource(ares, &sb))
+ return 1;
+
+ if (lookup->slave_addr != sb->slave_address)
+ return 1;
+
+ status = acpi_get_handle(lookup->dev_handle,
+sb->resource_source.string_ptr,
+&adapter_handle);
+ if (ACPI_FAILURE(status))
+ return 1;
+
+ adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
+ if (adapter)
+ intel_dsi->i2c_bus_num = adapter->nr;
+
+ return 1;
+}
+
static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
{
+ struct drm_device *dev = intel_dsi->base.base.dev;
+ struct i2c_adapter *adapter;
+ struct acpi_device *acpi_dev;
+ struct list_head resource_list;
+ struct i2c_adapter_lookup lookup;
+ struct i2c_msg msg;
+ int ret;
+ u8 vbt_i2c_bus_num = *(data + 2);
+ u16 slave_addr = *(u16 *)(data + 3);
+ u8 reg_offset = *(data + 5);
+ u8 payload_size = *(data + 6);
+ u8 *payload_data;
+
+ if (intel_dsi->i2c_bus_num < 0) {
+ intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
+
+ acpi_dev = ACPI_COMPANION(&dev->pdev->dev);
+ if (acpi_dev) {
+ memset(&lookup, 0, sizeof(lookup));
+ lookup.slave_addr = slave_addr;
+ lookup.intel_dsi = intel_dsi;
+ lookup.dev_handle = acpi_device_handle(acpi_dev);
+
+ INIT_LIST_HEAD(&resource_list);
+ acpi_dev_get_resources(acpi_dev, &resource_list,
+ i2c_adapter_lookup,
+ &lookup);
+ acpi_dev_free_resource_list(&resource_list);
+ }
+ }
+
+ adapter = i2c_get_adapter(intel_dsi->i2c_bus_num);
+ if (!adapter)
+ goto out;
This should never happen, so you should put a DRM_DEV_WARN here.
+
+ payload_data = kzalloc(payload_size + 1, GFP_KERNEL);
+ if (!payload_data)
+ got
[Intel-gfx] [PATCH] drm/i915/dsi: Parse the I2C element from the VBT MIPI sequence block
Parsing the i2c element is mainly done to transfer the payload from the
MIPI sequence block to the relevant slave device. In some cases, the
commands that are part of the payload can be used to turn on the backlight.
This patch is actually a refactored version of this old patch:
https://lists.freedesktop.org/archives/intel-gfx/2014-December/056897.html
In addition to the refactoring, the old patch is augmented by looking up
the i2c bus from ACPI NS instead of relying on the bus number provided
in the VBT.
Cc: Deepak M
Cc: Nabendu Maiti
Cc: Matt Roper
Cc: Bob Paauwe
Signed-off-by: Vivek Kasireddy
---
drivers/gpu/drm/i915/display/intel_dsi.h | 3 +
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 93
2 files changed, 96 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
b/drivers/gpu/drm/i915/display/intel_dsi.h
index b15be5814599..5651bc8aa5c2 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.h
+++ b/drivers/gpu/drm/i915/display/intel_dsi.h
@@ -68,6 +68,9 @@ struct intel_dsi {
/* number of DSI lanes */
unsigned int lane_count;
+ /* i2c bus associated with the slave device */
+ int i2c_bus_num;
+
/*
* video mode pixel format
*
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
index f90946c912ee..60441a5a3dba 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
@@ -83,6 +83,12 @@ static struct gpio_map vlv_gpio_table[] = {
{ VLV_GPIO_NC_11_PANEL1_BKLTCTL },
};
+struct i2c_adapter_lookup {
+ u16 slave_addr;
+ struct intel_dsi *intel_dsi;
+ acpi_handle dev_handle;
+};
+
#define CHV_GPIO_IDX_START_N 0
#define CHV_GPIO_IDX_START_E 73
#define CHV_GPIO_IDX_START_SW 100
@@ -375,8 +381,93 @@ static const u8 *mipi_exec_gpio(struct intel_dsi
*intel_dsi, const u8 *data)
return data;
}
+static int i2c_adapter_lookup(struct acpi_resource *ares, void *data)
+{
+ struct i2c_adapter_lookup *lookup = data;
+ struct intel_dsi *intel_dsi = lookup->intel_dsi;
+ struct acpi_resource_i2c_serialbus *sb;
+ struct i2c_adapter *adapter;
+ acpi_handle adapter_handle;
+ acpi_status status;
+
+ if (intel_dsi->i2c_bus_num >= 0 ||
+ !i2c_acpi_get_i2c_resource(ares, &sb))
+ return 1;
+
+ if (lookup->slave_addr != sb->slave_address)
+ return 1;
+
+ status = acpi_get_handle(lookup->dev_handle,
+sb->resource_source.string_ptr,
+&adapter_handle);
+ if (ACPI_FAILURE(status))
+ return 1;
+
+ adapter = i2c_acpi_find_adapter_by_handle(adapter_handle);
+ if (adapter)
+ intel_dsi->i2c_bus_num = adapter->nr;
+
+ return 1;
+}
+
static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
{
+ struct drm_device *dev = intel_dsi->base.base.dev;
+ struct i2c_adapter *adapter;
+ struct acpi_device *acpi_dev;
+ struct list_head resource_list;
+ struct i2c_adapter_lookup lookup;
+ struct i2c_msg msg;
+ int ret;
+ u8 vbt_i2c_bus_num = *(data + 2);
+ u16 slave_addr = *(u16 *)(data + 3);
+ u8 reg_offset = *(data + 5);
+ u8 payload_size = *(data + 6);
+ u8 *payload_data;
+
+ if (intel_dsi->i2c_bus_num < 0) {
+ intel_dsi->i2c_bus_num = vbt_i2c_bus_num;
+
+ acpi_dev = ACPI_COMPANION(&dev->pdev->dev);
+ if (acpi_dev) {
+ memset(&lookup, 0, sizeof(lookup));
+ lookup.slave_addr = slave_addr;
+ lookup.intel_dsi = intel_dsi;
+ lookup.dev_handle = acpi_device_handle(acpi_dev);
+
+ INIT_LIST_HEAD(&resource_list);
+ acpi_dev_get_resources(acpi_dev, &resource_list,
+ i2c_adapter_lookup,
+ &lookup);
+ acpi_dev_free_resource_list(&resource_list);
+ }
+ }
+
+ adapter = i2c_get_adapter(intel_dsi->i2c_bus_num);
+ if (!adapter)
+ goto out;
+
+ payload_data = kzalloc(payload_size + 1, GFP_KERNEL);
+ if (!payload_data)
+ goto out;
+
+ payload_data[0] = reg_offset;
+ memcpy(&payload_data[1], (data + 7), payload_size);
+
+ msg.addr = slave_addr;
+ msg.flags = 0;
+ msg.len = payload_size + 1;
+ msg.buf = payload_data;
+
+ ret = i2c_transfer(adapter, &msg, 1);
+ if (ret < 0)
+ DRM_ERROR("i2c transfer failed");
+
+ kfree(payload_data);
+ i2c_put_adapter(adapter);
+
+ return data + payload_size + 7;
+out:
DRM_DEBUG_KMS("Skipping I2C element execution\n");
