* [PATCH 0/5] drm/dp: i2c-over-aux short write support
@ 2015-03-13 16:13 ville.syrjala
2015-03-13 16:13 ` [PATCH 1/5] drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/ ville.syrjala
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: ville.syrjala @ 2015-03-13 16:13 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
This series tries to implement support for short i2c-over-aux writes.
I did notice that my monitor (HP ZR24w) does support DDC/CI so I was
able to do some writes, but I only saw native defers instead of i2c
defers/short acks. So I've not actually been able to test this.
Another complication with DDC/CI seems to be that the monitor doesn't
seem to like the default 100kHz bus speed. I had to drop it to 10kHz
to make it reliable (my dongle supports bus speed control fortunately).
Currently we have no standard way to configure the bus speed AFAICS,
so at least with this monitor DDC/CI is a bit useless.
I tried to fix up radeon and tegra too. gma500 I left alone since it's
not using dp i2c helper.
Ville Syrjälä (5):
drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/
drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE
drm/radeon: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE
drm/tegra: Handle I2C_WRITE_STATUS_UPDATE for address only writes
drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE
requests
drivers/gpu/drm/drm_dp_helper.c | 42 ++++++++++++++++++++++++++++++++----
drivers/gpu/drm/i915/intel_dp.c | 1 +
drivers/gpu/drm/radeon/atombios_dp.c | 1 +
drivers/gpu/drm/tegra/dpaux.c | 3 ++-
include/drm/drm_dp_helper.h | 2 +-
5 files changed, 43 insertions(+), 6 deletions(-)
--
2.0.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/5] drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/ 2015-03-13 16:13 [PATCH 0/5] drm/dp: i2c-over-aux short write support ville.syrjala @ 2015-03-13 16:13 ` ville.syrjala 2015-03-18 11:49 ` [Intel-gfx] " Jani Nikula 2015-03-13 16:13 ` [PATCH 2/5] drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE ville.syrjala ` (4 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: ville.syrjala @ 2015-03-13 16:13 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> Rename the I2C_STATUS request to I2C_WRITE_STATUS_UPDATE to match the spec. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/tegra/dpaux.c | 2 +- include/drm/drm_dp_helper.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c index d6b55e3..b1617af 100644 --- a/drivers/gpu/drm/tegra/dpaux.c +++ b/drivers/gpu/drm/tegra/dpaux.c @@ -152,7 +152,7 @@ static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux, break; - case DP_AUX_I2C_STATUS: + case DP_AUX_I2C_WRITE_STATUS_UPDATE: if (msg->request & DP_AUX_I2C_MOT) value |= DPAUX_DP_AUXCTL_CMD_MOT_RQ; else diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 523f04c..27301f5 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -46,7 +46,7 @@ #define DP_AUX_I2C_WRITE 0x0 #define DP_AUX_I2C_READ 0x1 -#define DP_AUX_I2C_STATUS 0x2 +#define DP_AUX_I2C_WRITE_STATUS_UPDATE 0x2 #define DP_AUX_I2C_MOT 0x4 #define DP_AUX_NATIVE_WRITE 0x8 #define DP_AUX_NATIVE_READ 0x9 -- 2.0.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Intel-gfx] [PATCH 1/5] drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/ 2015-03-13 16:13 ` [PATCH 1/5] drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/ ville.syrjala @ 2015-03-18 11:49 ` Jani Nikula 0 siblings, 0 replies; 13+ messages in thread From: Jani Nikula @ 2015-03-18 11:49 UTC (permalink / raw) To: ville.syrjala, dri-devel; +Cc: intel-gfx On Fri, 13 Mar 2015, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Rename the I2C_STATUS request to I2C_WRITE_STATUS_UPDATE to match the > spec. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/tegra/dpaux.c | 2 +- > include/drm/drm_dp_helper.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c > index d6b55e3..b1617af 100644 > --- a/drivers/gpu/drm/tegra/dpaux.c > +++ b/drivers/gpu/drm/tegra/dpaux.c > @@ -152,7 +152,7 @@ static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux, > > break; > > - case DP_AUX_I2C_STATUS: > + case DP_AUX_I2C_WRITE_STATUS_UPDATE: > if (msg->request & DP_AUX_I2C_MOT) > value |= DPAUX_DP_AUXCTL_CMD_MOT_RQ; > else > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 523f04c..27301f5 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -46,7 +46,7 @@ > > #define DP_AUX_I2C_WRITE 0x0 > #define DP_AUX_I2C_READ 0x1 > -#define DP_AUX_I2C_STATUS 0x2 > +#define DP_AUX_I2C_WRITE_STATUS_UPDATE 0x2 > #define DP_AUX_I2C_MOT 0x4 > #define DP_AUX_NATIVE_WRITE 0x8 > #define DP_AUX_NATIVE_READ 0x9 > -- > 2.0.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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/5] drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE 2015-03-13 16:13 [PATCH 0/5] drm/dp: i2c-over-aux short write support ville.syrjala 2015-03-13 16:13 ` [PATCH 1/5] drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/ ville.syrjala @ 2015-03-13 16:13 ` ville.syrjala 2015-03-18 11:52 ` Jani Nikula 2015-03-13 16:13 ` [PATCH 3/5] drm/radeon: " ville.syrjala ` (3 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: ville.syrjala @ 2015-03-13 16:13 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> When we get an i2c defer or short ack for i2c-over-aux write we need to switch to WRITE_STATUS_UPDATE to poll for the completion of the original request. i915 doesn't try to interpret wht request type apart from separating reads from writes, and so we should be able to treat this the same as a normal i2c write. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 33d5877..aed5f26 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -956,6 +956,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) switch (msg->request & ~DP_AUX_I2C_MOT) { case DP_AUX_NATIVE_WRITE: case DP_AUX_I2C_WRITE: + case DP_AUX_I2C_WRITE_STATUS_UPDATE: txsize = msg->size ? HEADER_SIZE + msg->size : BARE_ADDRESS_SIZE; rxsize = 1; -- 2.0.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/5] drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE 2015-03-13 16:13 ` [PATCH 2/5] drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE ville.syrjala @ 2015-03-18 11:52 ` Jani Nikula 0 siblings, 0 replies; 13+ messages in thread From: Jani Nikula @ 2015-03-18 11:52 UTC (permalink / raw) To: ville.syrjala, dri-devel; +Cc: intel-gfx On Fri, 13 Mar 2015, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > When we get an i2c defer or short ack for i2c-over-aux write we need > to switch to WRITE_STATUS_UPDATE to poll for the completion of the > original request. > > i915 doesn't try to interpret wht request type apart from separating > reads from writes, and so we should be able to treat this the same as > a normal i2c write. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> Probably needs a refresh due to context change due to my short writes patch though. > --- > drivers/gpu/drm/i915/intel_dp.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 33d5877..aed5f26 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -956,6 +956,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > switch (msg->request & ~DP_AUX_I2C_MOT) { > case DP_AUX_NATIVE_WRITE: > case DP_AUX_I2C_WRITE: > + case DP_AUX_I2C_WRITE_STATUS_UPDATE: > txsize = msg->size ? HEADER_SIZE + msg->size : BARE_ADDRESS_SIZE; > rxsize = 1; > > -- > 2.0.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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/5] drm/radeon: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE 2015-03-13 16:13 [PATCH 0/5] drm/dp: i2c-over-aux short write support ville.syrjala 2015-03-13 16:13 ` [PATCH 1/5] drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/ ville.syrjala 2015-03-13 16:13 ` [PATCH 2/5] drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE ville.syrjala @ 2015-03-13 16:13 ` ville.syrjala 2015-03-13 16:13 ` [PATCH 4/5] drm/tegra: Handle I2C_WRITE_STATUS_UPDATE for address only writes ville.syrjala ` (2 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: ville.syrjala @ 2015-03-13 16:13 UTC (permalink / raw) To: dri-devel; +Cc: Alex Deucher, intel-gfx, Christian König From: Ville Syrjälä <ville.syrjala@linux.intel.com> When we get an i2c defer or short ack for i2c-over-aux write we need to switch to WRITE_STATUS_UPDATE to poll for the completion of the original request. Looks like radeon doesn't do anything special with the request type, so hopefully just treating it the same as a i2c write is enough. Cc: Alex Deucher <alexander.deucher@amd.com> Cc: "Christian König" <christian.koenig@amd.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/radeon/atombios_dp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c index 8d74de8..9960b69 100644 --- a/drivers/gpu/drm/radeon/atombios_dp.c +++ b/drivers/gpu/drm/radeon/atombios_dp.c @@ -178,6 +178,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) switch (msg->request & ~DP_AUX_I2C_MOT) { case DP_AUX_NATIVE_WRITE: case DP_AUX_I2C_WRITE: + case DP_AUX_I2C_WRITE_STATUS_UPDATE: /* The atom implementation only supports writes with a max payload of * 12 bytes since it uses 4 bits for the total count (header + payload) * in the parameter space. The atom interface supports 16 byte -- 2.0.5 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/5] drm/tegra: Handle I2C_WRITE_STATUS_UPDATE for address only writes 2015-03-13 16:13 [PATCH 0/5] drm/dp: i2c-over-aux short write support ville.syrjala ` (2 preceding siblings ...) 2015-03-13 16:13 ` [PATCH 3/5] drm/radeon: " ville.syrjala @ 2015-03-13 16:13 ` ville.syrjala 2015-03-13 16:13 ` [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests ville.syrjala [not found] ` <CADnq5_NZyw-vY761GyrDapWjdY94Jk-2+6uoj906yG3BPWBsNQ@mail.gmail.com> 5 siblings, 0 replies; 13+ messages in thread From: ville.syrjala @ 2015-03-13 16:13 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx, Terje Bergström From: Ville Syrjälä <ville.syrjala@linux.intel.com> A address-only I2C_WRITE can't be replied with a short i2c ack, but I suppose it could be replied with an i2c defer. So the code should be prepared for an address-only I2C_WRITE_STATUS_UPDATE. Cc: Thierry Reding <thierry.reding@gmail.com> Cc: "Terje Bergström" <tbergstrom@nvidia.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/tegra/dpaux.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c index b1617af..a48687a 100644 --- a/drivers/gpu/drm/tegra/dpaux.c +++ b/drivers/gpu/drm/tegra/dpaux.c @@ -122,6 +122,7 @@ static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux, */ if (msg->size < 1) { switch (msg->request & ~DP_AUX_I2C_MOT) { + case DP_AUX_I2C_WRITE_STATUS_UPDATE: case DP_AUX_I2C_WRITE: case DP_AUX_I2C_READ: value = DPAUX_DP_AUXCTL_CMD_ADDRESS_ONLY; -- 2.0.5 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests 2015-03-13 16:13 [PATCH 0/5] drm/dp: i2c-over-aux short write support ville.syrjala ` (3 preceding siblings ...) 2015-03-13 16:13 ` [PATCH 4/5] drm/tegra: Handle I2C_WRITE_STATUS_UPDATE for address only writes ville.syrjala @ 2015-03-13 16:13 ` ville.syrjala 2015-03-14 5:02 ` shuang.he ` (2 more replies) [not found] ` <CADnq5_NZyw-vY761GyrDapWjdY94Jk-2+6uoj906yG3BPWBsNQ@mail.gmail.com> 5 siblings, 3 replies; 13+ messages in thread From: ville.syrjala @ 2015-03-13 16:13 UTC (permalink / raw) To: dri-devel; +Cc: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> When an i2c WRITE gets an i2c defer or short i2c ack reply, we are supposed to switch the request from I2C_WRITE to I2C_WRITE_STATUS_UPDATE when we continue to poll for the completion of the request. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/drm_dp_helper.c | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index d5368ea..4db81a6 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -422,6 +422,25 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter) I2C_FUNC_10BIT_ADDR; } +static void drm_dp_i2c_msg_set_request(struct drm_dp_aux_msg *msg, + const struct i2c_msg *i2c_msg) +{ + msg->request = (i2c_msg->flags & I2C_M_RD) ? + DP_AUX_I2C_READ : DP_AUX_I2C_WRITE; + msg->request |= DP_AUX_I2C_MOT; +} + +static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) +{ + /* + * In case of i2c defer or short i2c ack reply to a write, + * we need to switch to WRITE_STATUS_UPDATE to drain the + * rest of the message + */ + if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE) + msg->request |= DP_AUX_I2C_WRITE_STATUS_UPDATE; +} + /* * Transfer a single I2C-over-AUX message and handle various error conditions, * retrying the transaction as appropriate. It is assumed that the @@ -490,6 +509,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) * Both native ACK and I2C ACK replies received. We * can assume the transfer was successful. */ + if (ret != msg->size) + drm_dp_i2c_msg_write_status_update(msg); return ret; case DP_AUX_I2C_REPLY_NACK: @@ -501,6 +522,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) DRM_DEBUG_KMS("I2C defer\n"); aux->i2c_defer_count++; usleep_range(400, 500); + drm_dp_i2c_msg_write_status_update(msg); continue; default: @@ -566,10 +588,7 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, for (i = 0; i < num; i++) { msg.address = msgs[i].addr; - msg.request = (msgs[i].flags & I2C_M_RD) ? - DP_AUX_I2C_READ : - DP_AUX_I2C_WRITE; - msg.request |= DP_AUX_I2C_MOT; + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); /* Send a bare address packet to start the transaction. * Zero sized messages specify an address only (bare * address) transaction. @@ -577,6 +596,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, msg.buffer = NULL; msg.size = 0; err = drm_dp_i2c_do_msg(aux, &msg); + + /* + * Reset msg.request in case in case it got + * changed into a WRITE_STATUS_UPDATE. + */ + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); + if (err < 0) break; /* We want each transaction to be as large as possible, but @@ -589,6 +615,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, msg.size = min(transfer_size, msgs[i].len - j); err = drm_dp_i2c_drain_msg(aux, &msg); + + /* + * Reset msg.request in case in case it got + * changed into a WRITE_STATUS_UPDATE. + */ + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); + if (err < 0) break; transfer_size = err; -- 2.0.5 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests 2015-03-13 16:13 ` [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests ville.syrjala @ 2015-03-14 5:02 ` shuang.he 2015-03-17 15:22 ` Jani Nikula 2015-03-18 12:05 ` Jani Nikula 2 siblings, 0 replies; 13+ messages in thread From: shuang.he @ 2015-03-14 5:02 UTC (permalink / raw) To: shuang.he, ethan.gao, intel-gfx, ville.syrjala Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) Task id: 5948 -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 303/303 303/303 SNB -1 279/279 278/279 IVB 343/343 343/343 BYT 287/287 287/287 HSW 363/363 363/363 BDW 308/308 308/308 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied *SNB igt_gem_persistent_relocs_forked-interruptible-faulting-reloc-thrash-inactive PASS(2) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests 2015-03-13 16:13 ` [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests ville.syrjala 2015-03-14 5:02 ` shuang.he @ 2015-03-17 15:22 ` Jani Nikula 2015-03-19 9:21 ` Ville Syrjälä 2015-03-18 12:05 ` Jani Nikula 2 siblings, 1 reply; 13+ messages in thread From: Jani Nikula @ 2015-03-17 15:22 UTC (permalink / raw) To: ville.syrjala, dri-devel; +Cc: intel-gfx On Fri, 13 Mar 2015, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > When an i2c WRITE gets an i2c defer or short i2c ack reply, we are > supposed to switch the request from I2C_WRITE to I2C_WRITE_STATUS_UPDATE > when we continue to poll for the completion of the request. As I said IRL, it's a bit unfortunate that setting and resetting of DP_AUX_I2C_WRITE_STATUS_UPDATE happen at different levels of the abstraction. But I can live with that. We also have another problem with reporting short writes in i915, hopefully fixed by [1]. Before that gets fixed, can't really r-b. BR, Jani. [1] http://mid.gmane.org/1426605534-5780-1-git-send-email-jani.nikula@intel.com > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_dp_helper.c | 41 +++++++++++++++++++++++++++++++++++++---- > 1 file changed, 37 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index d5368ea..4db81a6 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -422,6 +422,25 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter) > I2C_FUNC_10BIT_ADDR; > } > > +static void drm_dp_i2c_msg_set_request(struct drm_dp_aux_msg *msg, > + const struct i2c_msg *i2c_msg) > +{ > + msg->request = (i2c_msg->flags & I2C_M_RD) ? > + DP_AUX_I2C_READ : DP_AUX_I2C_WRITE; > + msg->request |= DP_AUX_I2C_MOT; > +} > + > +static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) > +{ > + /* > + * In case of i2c defer or short i2c ack reply to a write, > + * we need to switch to WRITE_STATUS_UPDATE to drain the > + * rest of the message > + */ > + if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE) > + msg->request |= DP_AUX_I2C_WRITE_STATUS_UPDATE; > +} > + > /* > * Transfer a single I2C-over-AUX message and handle various error conditions, > * retrying the transaction as appropriate. It is assumed that the > @@ -490,6 +509,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > * Both native ACK and I2C ACK replies received. We > * can assume the transfer was successful. > */ > + if (ret != msg->size) > + drm_dp_i2c_msg_write_status_update(msg); > return ret; > > case DP_AUX_I2C_REPLY_NACK: > @@ -501,6 +522,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > DRM_DEBUG_KMS("I2C defer\n"); > aux->i2c_defer_count++; > usleep_range(400, 500); > + drm_dp_i2c_msg_write_status_update(msg); > continue; > > default: > @@ -566,10 +588,7 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > > for (i = 0; i < num; i++) { > msg.address = msgs[i].addr; > - msg.request = (msgs[i].flags & I2C_M_RD) ? > - DP_AUX_I2C_READ : > - DP_AUX_I2C_WRITE; > - msg.request |= DP_AUX_I2C_MOT; > + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > /* Send a bare address packet to start the transaction. > * Zero sized messages specify an address only (bare > * address) transaction. > @@ -577,6 +596,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > msg.buffer = NULL; > msg.size = 0; > err = drm_dp_i2c_do_msg(aux, &msg); > + > + /* > + * Reset msg.request in case in case it got > + * changed into a WRITE_STATUS_UPDATE. > + */ > + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > + > if (err < 0) > break; > /* We want each transaction to be as large as possible, but > @@ -589,6 +615,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > msg.size = min(transfer_size, msgs[i].len - j); > > err = drm_dp_i2c_drain_msg(aux, &msg); > + > + /* > + * Reset msg.request in case in case it got > + * changed into a WRITE_STATUS_UPDATE. > + */ > + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > + > if (err < 0) > break; > transfer_size = err; > -- > 2.0.5 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests 2015-03-17 15:22 ` Jani Nikula @ 2015-03-19 9:21 ` Ville Syrjälä 0 siblings, 0 replies; 13+ messages in thread From: Ville Syrjälä @ 2015-03-19 9:21 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx, dri-devel On Tue, Mar 17, 2015 at 05:22:08PM +0200, Jani Nikula wrote: > On Fri, 13 Mar 2015, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > When an i2c WRITE gets an i2c defer or short i2c ack reply, we are > > supposed to switch the request from I2C_WRITE to I2C_WRITE_STATUS_UPDATE > > when we continue to poll for the completion of the request. > > As I said IRL, it's a bit unfortunate that setting and resetting of > DP_AUX_I2C_WRITE_STATUS_UPDATE happen at different levels of the > abstraction. But I can live with that. > > We also have another problem with reporting short writes in i915, > hopefully fixed by [1]. Before that gets fixed, can't really r-b. > > BR, > Jani. > > > [1] http://mid.gmane.org/1426605534-5780-1-git-send-email-jani.nikula@intel.com BTW I notice now that radeon seems to have that same problem. I won't try to fix that, so just a FYI for the radeon folks... > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/drm_dp_helper.c | 41 +++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 37 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > > index d5368ea..4db81a6 100644 > > --- a/drivers/gpu/drm/drm_dp_helper.c > > +++ b/drivers/gpu/drm/drm_dp_helper.c > > @@ -422,6 +422,25 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter) > > I2C_FUNC_10BIT_ADDR; > > } > > > > +static void drm_dp_i2c_msg_set_request(struct drm_dp_aux_msg *msg, > > + const struct i2c_msg *i2c_msg) > > +{ > > + msg->request = (i2c_msg->flags & I2C_M_RD) ? > > + DP_AUX_I2C_READ : DP_AUX_I2C_WRITE; > > + msg->request |= DP_AUX_I2C_MOT; > > +} > > + > > +static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) > > +{ > > + /* > > + * In case of i2c defer or short i2c ack reply to a write, > > + * we need to switch to WRITE_STATUS_UPDATE to drain the > > + * rest of the message > > + */ > > + if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE) > > + msg->request |= DP_AUX_I2C_WRITE_STATUS_UPDATE; > > +} > > + > > /* > > * Transfer a single I2C-over-AUX message and handle various error conditions, > > * retrying the transaction as appropriate. It is assumed that the > > @@ -490,6 +509,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > > * Both native ACK and I2C ACK replies received. We > > * can assume the transfer was successful. > > */ > > + if (ret != msg->size) > > + drm_dp_i2c_msg_write_status_update(msg); > > return ret; > > > > case DP_AUX_I2C_REPLY_NACK: > > @@ -501,6 +522,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > > DRM_DEBUG_KMS("I2C defer\n"); > > aux->i2c_defer_count++; > > usleep_range(400, 500); > > + drm_dp_i2c_msg_write_status_update(msg); > > continue; > > > > default: > > @@ -566,10 +588,7 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > > > > for (i = 0; i < num; i++) { > > msg.address = msgs[i].addr; > > - msg.request = (msgs[i].flags & I2C_M_RD) ? > > - DP_AUX_I2C_READ : > > - DP_AUX_I2C_WRITE; > > - msg.request |= DP_AUX_I2C_MOT; > > + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > > /* Send a bare address packet to start the transaction. > > * Zero sized messages specify an address only (bare > > * address) transaction. > > @@ -577,6 +596,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > > msg.buffer = NULL; > > msg.size = 0; > > err = drm_dp_i2c_do_msg(aux, &msg); > > + > > + /* > > + * Reset msg.request in case in case it got > > + * changed into a WRITE_STATUS_UPDATE. > > + */ > > + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > > + > > if (err < 0) > > break; > > /* We want each transaction to be as large as possible, but > > @@ -589,6 +615,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > > msg.size = min(transfer_size, msgs[i].len - j); > > > > err = drm_dp_i2c_drain_msg(aux, &msg); > > + > > + /* > > + * Reset msg.request in case in case it got > > + * changed into a WRITE_STATUS_UPDATE. > > + */ > > + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > > + > > if (err < 0) > > break; > > transfer_size = err; > > -- > > 2.0.5 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests 2015-03-13 16:13 ` [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests ville.syrjala 2015-03-14 5:02 ` shuang.he 2015-03-17 15:22 ` Jani Nikula @ 2015-03-18 12:05 ` Jani Nikula 2 siblings, 0 replies; 13+ messages in thread From: Jani Nikula @ 2015-03-18 12:05 UTC (permalink / raw) To: ville.syrjala, dri-devel; +Cc: intel-gfx On Fri, 13 Mar 2015, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > When an i2c WRITE gets an i2c defer or short i2c ack reply, we are > supposed to switch the request from I2C_WRITE to I2C_WRITE_STATUS_UPDATE > when we continue to poll for the completion of the request. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_dp_helper.c | 41 +++++++++++++++++++++++++++++++++++++---- > 1 file changed, 37 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index d5368ea..4db81a6 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -422,6 +422,25 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter) > I2C_FUNC_10BIT_ADDR; > } > > +static void drm_dp_i2c_msg_set_request(struct drm_dp_aux_msg *msg, > + const struct i2c_msg *i2c_msg) > +{ > + msg->request = (i2c_msg->flags & I2C_M_RD) ? > + DP_AUX_I2C_READ : DP_AUX_I2C_WRITE; > + msg->request |= DP_AUX_I2C_MOT; > +} > + > +static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) > +{ > + /* > + * In case of i2c defer or short i2c ack reply to a write, > + * we need to switch to WRITE_STATUS_UPDATE to drain the > + * rest of the message > + */ > + if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE) > + msg->request |= DP_AUX_I2C_WRITE_STATUS_UPDATE; This only works because DP_AUX_I2C_WRITE == 0, and it may not be obvious to the casual reader that DP_AUX_I2C_WRITE_STATUS_UPDATE is a replacement not an addition to DP_AUX_I2C_WRITE. Whether you fix that or not, this is Reviewed-by: Jani Nikula <jani.nikula@intel.com> > +} > + > /* > * Transfer a single I2C-over-AUX message and handle various error conditions, > * retrying the transaction as appropriate. It is assumed that the > @@ -490,6 +509,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > * Both native ACK and I2C ACK replies received. We > * can assume the transfer was successful. > */ > + if (ret != msg->size) > + drm_dp_i2c_msg_write_status_update(msg); > return ret; > > case DP_AUX_I2C_REPLY_NACK: > @@ -501,6 +522,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > DRM_DEBUG_KMS("I2C defer\n"); > aux->i2c_defer_count++; > usleep_range(400, 500); > + drm_dp_i2c_msg_write_status_update(msg); > continue; > > default: > @@ -566,10 +588,7 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > > for (i = 0; i < num; i++) { > msg.address = msgs[i].addr; > - msg.request = (msgs[i].flags & I2C_M_RD) ? > - DP_AUX_I2C_READ : > - DP_AUX_I2C_WRITE; > - msg.request |= DP_AUX_I2C_MOT; > + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > /* Send a bare address packet to start the transaction. > * Zero sized messages specify an address only (bare > * address) transaction. > @@ -577,6 +596,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > msg.buffer = NULL; > msg.size = 0; > err = drm_dp_i2c_do_msg(aux, &msg); > + > + /* > + * Reset msg.request in case in case it got > + * changed into a WRITE_STATUS_UPDATE. > + */ > + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > + > if (err < 0) > break; > /* We want each transaction to be as large as possible, but > @@ -589,6 +615,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > msg.size = min(transfer_size, msgs[i].len - j); > > err = drm_dp_i2c_drain_msg(aux, &msg); > + > + /* > + * Reset msg.request in case in case it got > + * changed into a WRITE_STATUS_UPDATE. > + */ > + drm_dp_i2c_msg_set_request(&msg, &msgs[i]); > + > if (err < 0) > break; > transfer_size = err; > -- > 2.0.5 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <CADnq5_NZyw-vY761GyrDapWjdY94Jk-2+6uoj906yG3BPWBsNQ@mail.gmail.com>]
* Fwd: [PATCH 0/5] drm/dp: i2c-over-aux short write support [not found] ` <CADnq5_NZyw-vY761GyrDapWjdY94Jk-2+6uoj906yG3BPWBsNQ@mail.gmail.com> @ 2015-03-18 13:26 ` Alex Deucher 0 siblings, 0 replies; 13+ messages in thread From: Alex Deucher @ 2015-03-18 13:26 UTC (permalink / raw) To: Maling list - DRI developers fwd to the list. ---------- Forwarded message ---------- From: Alex Deucher <alexdeucher@gmail.com> Date: Mon, Mar 16, 2015 at 9:49 AM Subject: Re: [PATCH 0/5] drm/dp: i2c-over-aux short write support To: Ville Syrjälä <ville.syrjala@linux.intel.com> On Fri, Mar 13, 2015 at 12:13 PM, <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > This series tries to implement support for short i2c-over-aux writes. > > I did notice that my monitor (HP ZR24w) does support DDC/CI so I was > able to do some writes, but I only saw native defers instead of i2c > defers/short acks. So I've not actually been able to test this. > > Another complication with DDC/CI seems to be that the monitor doesn't > seem to like the default 100kHz bus speed. I had to drop it to 10kHz > to make it reliable (my dongle supports bus speed control fortunately). > Currently we have no standard way to configure the bus speed AFAICS, > so at least with this monitor DDC/CI is a bit useless. > > I tried to fix up radeon and tegra too. gma500 I left alone since it's > not using dp i2c helper. > > Ville Syrjälä (5): > drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/ > drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE > drm/radeon: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE > drm/tegra: Handle I2C_WRITE_STATUS_UPDATE for address only writes > drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE > requests Patches 1, 3, 5 are: Acked-by: Alex Deucher <alexander.deucher@amd.com> > > drivers/gpu/drm/drm_dp_helper.c | 42 ++++++++++++++++++++++++++++++++---- > drivers/gpu/drm/i915/intel_dp.c | 1 + > drivers/gpu/drm/radeon/atombios_dp.c | 1 + > drivers/gpu/drm/tegra/dpaux.c | 3 ++- > include/drm/drm_dp_helper.h | 2 +- > 5 files changed, 43 insertions(+), 6 deletions(-) > > -- > 2.0.5 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-03-19 9:21 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-13 16:13 [PATCH 0/5] drm/dp: i2c-over-aux short write support ville.syrjala
2015-03-13 16:13 ` [PATCH 1/5] drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/ ville.syrjala
2015-03-18 11:49 ` [Intel-gfx] " Jani Nikula
2015-03-13 16:13 ` [PATCH 2/5] drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE ville.syrjala
2015-03-18 11:52 ` Jani Nikula
2015-03-13 16:13 ` [PATCH 3/5] drm/radeon: " ville.syrjala
2015-03-13 16:13 ` [PATCH 4/5] drm/tegra: Handle I2C_WRITE_STATUS_UPDATE for address only writes ville.syrjala
2015-03-13 16:13 ` [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests ville.syrjala
2015-03-14 5:02 ` shuang.he
2015-03-17 15:22 ` Jani Nikula
2015-03-19 9:21 ` Ville Syrjälä
2015-03-18 12:05 ` Jani Nikula
[not found] ` <CADnq5_NZyw-vY761GyrDapWjdY94Jk-2+6uoj906yG3BPWBsNQ@mail.gmail.com>
2015-03-18 13:26 ` Fwd: [PATCH 0/5] drm/dp: i2c-over-aux short write support Alex Deucher
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.