| Message ID | 1441841070-11532-2-git-send-email-m.deepak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 10 Sep 2015, Deepak M <m.deepak@intel.com> wrote: > From: vkorjani <vikas.korjani@intel.com> > > New sequence element for i2c is been added in the > mipi sequence block of the VBT. This patch parses > and executes the i2c sequence. > > v2: Add i2c_put_adapter call(Jani), rebase > > Signed-off-by: vkorjani <vikas.korjani@intel.com> > Signed-off-by: Deepak M <m.deepak@intel.com> > --- > drivers/gpu/drm/i915/intel_bios.c | 6 +++ > drivers/gpu/drm/i915/intel_bios.h | 1 + > drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 61 ++++++++++++++++++++++++++++ > 3 files changed, 68 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index c8acc29..0bf0942 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -714,6 +714,12 @@ static u8 *goto_next_sequence(u8 *data, int *size) > > data += 3; > break; > + case MIPI_SEQ_ELEM_I2C: > + /* skip by this element payload size */ > + data += 7; > + len = *data; > + data += len + 1; > + break; > default: > DRM_ERROR("Unknown element\n"); > return NULL; > diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h > index 1b7417e..21a7f3f 100644 > --- a/drivers/gpu/drm/i915/intel_bios.h > +++ b/drivers/gpu/drm/i915/intel_bios.h > @@ -956,6 +956,7 @@ enum mipi_seq_element { > MIPI_SEQ_ELEM_SEND_PKT, > MIPI_SEQ_ELEM_DELAY, > MIPI_SEQ_ELEM_GPIO, > + MIPI_SEQ_ELEM_I2C, > MIPI_SEQ_ELEM_STATUS, Side note, MIPI_SEQ_ELEM_STATUS doesn't seem to be in spec. > MIPI_SEQ_ELEM_MAX > }; > diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > index a5e99ac..9989f61 100644 > --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > @@ -31,6 +31,7 @@ > #include <drm/drm_panel.h> > #include <linux/slab.h> > #include <video/mipi_display.h> > +#include <linux/i2c.h> > #include <asm/intel-mid.h> > #include <video/mipi_display.h> > #include "i915_drv.h" > @@ -104,6 +105,65 @@ static struct gpio_table gtable[] = { > { GPIO_NC_11_PCONF0, GPIO_NC_11_PAD, 0} > }; > > +static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data) > +{ > + struct i2c_adapter *adapter; > + int ret; > + u8 reg_offset, payload_size, retries = 5; > + struct i2c_msg msg; > + u8 *transmit_buffer = NULL; > + u8 flag = *data++; > + u8 index = *data++; > + u8 bus_number = *data++; > + u16 slave_add = *(u16 *)(data); > + > + data = data + 2; > + reg_offset = *data++; > + payload_size = *data++; > + > + adapter = i2c_get_adapter(bus_number); > + > + if (!adapter) { > + DRM_ERROR("i2c_get_adapter(%u) failed, index:%u flag: %u\n", > + (bus_number + 1), index, flag); Why do you log bus_number + 1 instead of what was actually tried? I don't see why the flag/index are useful for in the message, as they're not relevant to the i2c_get_adapter failing. > + goto out; > + } > + > + transmit_buffer = kmalloc(1 + payload_size, GFP_TEMPORARY); > + > + if (!transmit_buffer) > + goto out; > + > + transmit_buffer[0] = reg_offset; > + memcpy(&transmit_buffer[1], data, (size_t)payload_size); Why do you need the cast? > + > + msg.addr = slave_add; > + msg.flags = 0; > + msg.len = payload_size + 1; > + msg.buf = &transmit_buffer[0]; > + > + do { > + ret = i2c_transfer(adapter, &msg, 1); > + if (ret == 1) > + break; > + else if (ret == -EAGAIN) > + usleep_range(1000, 2500); > + else { > + DRM_ERROR("i2c transfer failed, error code:%d", ret); \n missing. > + break; > + } > + } while (retries--); > + > + if (retries == 0) Due to the way you post-decrement retries, you may end up here even if the transfer succeeds on the last try. You may also end up printing two error messages, if i2c_transfer fails on the last try. > + DRM_ERROR("i2c transfer failed, error code:%d", ret); \n missing. > +out: > + kfree(transmit_buffer); > + i2c_put_adapter(adapter); > + > + data = data + payload_size; > + return data; > +} > + > static inline enum port intel_dsi_seq_port_to_port(u8 port) > { > return port ? PORT_C : PORT_A; > @@ -236,6 +296,7 @@ static const fn_mipi_elem_exec exec_elem[] = { > mipi_exec_send_packet, > mipi_exec_delay, > mipi_exec_gpio, > + mipi_exec_i2c, > NULL, /* status read; later */ > }; > > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>-----Original Message----- >From: Jani Nikula [mailto:jani.nikula@linux.intel.com] >Sent: Thursday, September 17, 2015 2:48 PM >To: Deepak, M; intel-gfx@lists.freedesktop.org >Cc: Deepak, M >Subject: Re: [Intel-gfx] [MIPI SEQ PARSING v2 PATCH 01/11] drm/i915: Adding >the parsing logic for the i2c element > >On Thu, 10 Sep 2015, Deepak M <m.deepak@intel.com> wrote: >> From: vkorjani <vikas.korjani@intel.com> >> >> New sequence element for i2c is been added in the mipi sequence block >> of the VBT. This patch parses and executes the i2c sequence. >> >> v2: Add i2c_put_adapter call(Jani), rebase >> >> Signed-off-by: vkorjani <vikas.korjani@intel.com> >> Signed-off-by: Deepak M <m.deepak@intel.com> >> --- >> drivers/gpu/drm/i915/intel_bios.c | 6 +++ >> drivers/gpu/drm/i915/intel_bios.h | 1 + >> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 61 >++++++++++++++++++++++++++++ >> 3 files changed, 68 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_bios.c >> b/drivers/gpu/drm/i915/intel_bios.c >> index c8acc29..0bf0942 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.c >> +++ b/drivers/gpu/drm/i915/intel_bios.c >> @@ -714,6 +714,12 @@ static u8 *goto_next_sequence(u8 *data, int >> *size) >> >> data += 3; >> break; >> + case MIPI_SEQ_ELEM_I2C: >> + /* skip by this element payload size */ >> + data += 7; >> + len = *data; >> + data += len + 1; >> + break; >> default: >> DRM_ERROR("Unknown element\n"); >> return NULL; >> diff --git a/drivers/gpu/drm/i915/intel_bios.h >> b/drivers/gpu/drm/i915/intel_bios.h >> index 1b7417e..21a7f3f 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.h >> +++ b/drivers/gpu/drm/i915/intel_bios.h >> @@ -956,6 +956,7 @@ enum mipi_seq_element { >> MIPI_SEQ_ELEM_SEND_PKT, >> MIPI_SEQ_ELEM_DELAY, >> MIPI_SEQ_ELEM_GPIO, >> + MIPI_SEQ_ELEM_I2C, >> MIPI_SEQ_ELEM_STATUS, > >Side note, MIPI_SEQ_ELEM_STATUS doesn't seem to be in spec. > > >> MIPI_SEQ_ELEM_MAX >> }; >> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> index a5e99ac..9989f61 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> @@ -31,6 +31,7 @@ >> #include <drm/drm_panel.h> >> #include <linux/slab.h> >> #include <video/mipi_display.h> >> +#include <linux/i2c.h> >> #include <asm/intel-mid.h> >> #include <video/mipi_display.h> >> #include "i915_drv.h" >> @@ -104,6 +105,65 @@ static struct gpio_table gtable[] = { >> { GPIO_NC_11_PCONF0, GPIO_NC_11_PAD, 0} }; >> >> +static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 >> +*data) { >> + struct i2c_adapter *adapter; >> + int ret; >> + u8 reg_offset, payload_size, retries = 5; >> + struct i2c_msg msg; >> + u8 *transmit_buffer = NULL; >> + u8 flag = *data++; >> + u8 index = *data++; >> + u8 bus_number = *data++; >> + u16 slave_add = *(u16 *)(data); >> + >> + data = data + 2; >> + reg_offset = *data++; >> + payload_size = *data++; >> + >> + adapter = i2c_get_adapter(bus_number); >> + >> + if (!adapter) { >> + DRM_ERROR("i2c_get_adapter(%u) failed, index:%u flag: >%u\n", >> + (bus_number + 1), index, flag); > >Why do you log bus_number + 1 instead of what was actually tried? [Deepak M] Will fix this. > >I don't see why the flag/index are useful for in the message, as they're not >relevant to the i2c_get_adapter failing. > [Deepak M] Flag field is reserved and then the index is used for windows, for Linux these two can be skipped. Will not try to declare flag and index variable. >> + goto out; >> + } >> + >> + transmit_buffer = kmalloc(1 + payload_size, GFP_TEMPORARY); >> + >> + if (!transmit_buffer) >> + goto out; >> + >> + transmit_buffer[0] = reg_offset; >> + memcpy(&transmit_buffer[1], data, (size_t)payload_size); > >Why do you need the cast? > >> + >> + msg.addr = slave_add; >> + msg.flags = 0; >> + msg.len = payload_size + 1; >> + msg.buf = &transmit_buffer[0]; >> + >> + do { >> + ret = i2c_transfer(adapter, &msg, 1); >> + if (ret == 1) >> + break; >> + else if (ret == -EAGAIN) >> + usleep_range(1000, 2500); >> + else { >> + DRM_ERROR("i2c transfer failed, error code:%d", ret); > >\n missing. > >> + break; >> + } >> + } while (retries--); >> + >> + if (retries == 0) > >Due to the way you post-decrement retries, you may end up here even if the >transfer succeeds on the last try. > >You may also end up printing two error messages, if i2c_transfer fails on the >last try. [Deepak M] okay, Will handle this. > >> + DRM_ERROR("i2c transfer failed, error code:%d", ret); > >\n missing. > >> +out: >> + kfree(transmit_buffer); >> + i2c_put_adapter(adapter); >> + >> + data = data + payload_size; >> + return data; >> +} >> + >> static inline enum port intel_dsi_seq_port_to_port(u8 port) { >> return port ? PORT_C : PORT_A; >> @@ -236,6 +296,7 @@ static const fn_mipi_elem_exec exec_elem[] = { >> mipi_exec_send_packet, >> mipi_exec_delay, >> mipi_exec_gpio, >> + mipi_exec_i2c, >> NULL, /* status read; later */ >> }; >> >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index c8acc29..0bf0942 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -714,6 +714,12 @@ static u8 *goto_next_sequence(u8 *data, int *size) data += 3; break; + case MIPI_SEQ_ELEM_I2C: + /* skip by this element payload size */ + data += 7; + len = *data; + data += len + 1; + break; default: DRM_ERROR("Unknown element\n"); return NULL; diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h index 1b7417e..21a7f3f 100644 --- a/drivers/gpu/drm/i915/intel_bios.h +++ b/drivers/gpu/drm/i915/intel_bios.h @@ -956,6 +956,7 @@ enum mipi_seq_element { MIPI_SEQ_ELEM_SEND_PKT, MIPI_SEQ_ELEM_DELAY, MIPI_SEQ_ELEM_GPIO, + MIPI_SEQ_ELEM_I2C, MIPI_SEQ_ELEM_STATUS, MIPI_SEQ_ELEM_MAX }; diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c index a5e99ac..9989f61 100644 --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c @@ -31,6 +31,7 @@ #include <drm/drm_panel.h> #include <linux/slab.h> #include <video/mipi_display.h> +#include <linux/i2c.h> #include <asm/intel-mid.h> #include <video/mipi_display.h> #include "i915_drv.h" @@ -104,6 +105,65 @@ static struct gpio_table gtable[] = { { GPIO_NC_11_PCONF0, GPIO_NC_11_PAD, 0} }; +static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data) +{ + struct i2c_adapter *adapter; + int ret; + u8 reg_offset, payload_size, retries = 5; + struct i2c_msg msg; + u8 *transmit_buffer = NULL; + u8 flag = *data++; + u8 index = *data++; + u8 bus_number = *data++; + u16 slave_add = *(u16 *)(data); + + data = data + 2; + reg_offset = *data++; + payload_size = *data++; + + adapter = i2c_get_adapter(bus_number); + + if (!adapter) { + DRM_ERROR("i2c_get_adapter(%u) failed, index:%u flag: %u\n", + (bus_number + 1), index, flag); + goto out; + } + + transmit_buffer = kmalloc(1 + payload_size, GFP_TEMPORARY); + + if (!transmit_buffer) + goto out; + + transmit_buffer[0] = reg_offset; + memcpy(&transmit_buffer[1], data, (size_t)payload_size); + + msg.addr = slave_add; + msg.flags = 0; + msg.len = payload_size + 1; + msg.buf = &transmit_buffer[0]; + + do { + ret = i2c_transfer(adapter, &msg, 1); + if (ret == 1) + break; + else if (ret == -EAGAIN) + usleep_range(1000, 2500); + else { + DRM_ERROR("i2c transfer failed, error code:%d", ret); + break; + } + } while (retries--); + + if (retries == 0) + DRM_ERROR("i2c transfer failed, error code:%d", ret); +out: + kfree(transmit_buffer); + i2c_put_adapter(adapter); + + data = data + payload_size; + return data; +} + static inline enum port intel_dsi_seq_port_to_port(u8 port) { return port ? PORT_C : PORT_A; @@ -236,6 +296,7 @@ static const fn_mipi_elem_exec exec_elem[] = { mipi_exec_send_packet, mipi_exec_delay, mipi_exec_gpio, + mipi_exec_i2c, NULL, /* status read; later */ };