diff mbox

drm/i915: support address only i2c-over-aux transactions

Message ID 1396863445-30958-1-git-send-email-jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jani Nikula April 7, 2014, 9:37 a.m. UTC
To support bare address requests used by the drm dp helpers.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

Hi Alex, similar to Thierry's patch for i915.

BR,
Jani.
---
 drivers/gpu/drm/i915/intel_dp.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Alex Deucher April 7, 2014, 2:35 p.m. UTC | #1
On Mon, Apr 7, 2014 at 5:37 AM, Jani Nikula <jani.nikula@intel.com> wrote:
> To support bare address requests used by the drm dp helpers.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ---
>
> Hi Alex, similar to Thierry's patch for i915.
>

Looks good to me.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Do you want me to add this to the patch set?

Alex

> BR,
> Jani.
> ---
>  drivers/gpu/drm/i915/intel_dp.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e48d47ced57b..b435d07fbc08 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -575,7 +575,8 @@ out:
>         return ret;
>  }
>
> -#define HEADER_SIZE    4
> +#define BARE_ADDRESS_SIZE      3
> +#define HEADER_SIZE            (BARE_ADDRESS_SIZE + 1)
>  static ssize_t
>  intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  {
> @@ -592,7 +593,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:
> -               txsize = HEADER_SIZE + msg->size;
> +               txsize = msg->size ? HEADER_SIZE + msg->size : BARE_ADDRESS_SIZE;
>                 rxsize = 1;
>
>                 if (WARN_ON(txsize > 20))
> @@ -611,7 +612,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>
>         case DP_AUX_NATIVE_READ:
>         case DP_AUX_I2C_READ:
> -               txsize = HEADER_SIZE;
> +               txsize = msg->size ? HEADER_SIZE : BARE_ADDRESS_SIZE;
>                 rxsize = msg->size + 1;
>
>                 if (WARN_ON(rxsize > 20))
> --
> 1.7.9.5
>
Daniel Vetter April 7, 2014, 8 p.m. UTC | #2
On Mon, Apr 7, 2014 at 4:35 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Mon, Apr 7, 2014 at 5:37 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>> To support bare address requests used by the drm dp helpers.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> ---
>>
>> Hi Alex, similar to Thierry's patch for i915.
>>
>
> Looks good to me.
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Do you want me to add this to the patch set?

Afaict we've done an unconditional msg_bytes = send_bytes + 4; in the
i915 driver before the conversion to the dp aux helper, which is why
I've slapped an r-b onto your patch without asking for a i915
adjustement. Otoh we have a pile of bugs for dp dongles still. Imo my
preferred approach would be to get the helper + radeon stuff in and
then hawk the i915 patch to a bunch of bug reporter and see whether it
sticks. Ofc if we already know that we need it it would be best to
merge it in one pull together with all your other patches.

Jani?
-Daniel
Jani Nikula April 8, 2014, 6:58 a.m. UTC | #3
On Mon, 07 Apr 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Apr 7, 2014 at 4:35 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Mon, Apr 7, 2014 at 5:37 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>>> To support bare address requests used by the drm dp helpers.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>
>>> ---
>>>
>>> Hi Alex, similar to Thierry's patch for i915.
>>>
>>
>> Looks good to me.
>>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> Do you want me to add this to the patch set?
>
> Afaict we've done an unconditional msg_bytes = send_bytes + 4; in the
> i915 driver before the conversion to the dp aux helper, which is why
> I've slapped an r-b onto your patch without asking for a i915
> adjustement. Otoh we have a pile of bugs for dp dongles still. Imo my
> preferred approach would be to get the helper + radeon stuff in and
> then hawk the i915 patch to a bunch of bug reporter and see whether it
> sticks. Ofc if we already know that we need it it would be best to
> merge it in one pull together with all your other patches.
>
> Jani?

Before the conversion to dp aux helpers there was a switch case [1] that
ended up in msg_bytes = 3 for address only start/stop messages
(MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
but without my patch, we'd send the four byte header but with 0 - 1 =
0xff in txbuf[3]. I'm pretty sure breakage would follow.

Thus I'd like to have my patch in before the dp aux helpers use
msg->size == 0 for address only messages.

Daniel, convinced yet?

BR,
Jani.


[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/i915/intel_dp.c?id=v3.14#n654

[2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_dp_helper.c?id=v3.14#n59


> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Vetter April 8, 2014, 8:03 a.m. UTC | #4
On Tue, Apr 8, 2014 at 8:58 AM, Jani Nikula <jani.nikula@intel.com> wrote:
> Before the conversion to dp aux helpers there was a switch case [1] that
> ended up in msg_bytes = 3 for address only start/stop messages
> (MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
> but without my patch, we'd send the four byte header but with 0 - 1 =
> 0xff in txbuf[3]. I'm pretty sure breakage would follow.
>
> Thus I'd like to have my patch in before the dp aux helpers use
> msg->size == 0 for address only messages.
>
> Daniel, convinced yet?

Indeed, I've been blinding. Acked for merging through radeon trees
together with the other patches as a fixup for 3.15.
-Daniel
Alex Deucher April 8, 2014, 1:04 p.m. UTC | #5
On Tue, Apr 8, 2014 at 4:03 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Apr 8, 2014 at 8:58 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>> Before the conversion to dp aux helpers there was a switch case [1] that
>> ended up in msg_bytes = 3 for address only start/stop messages
>> (MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
>> but without my patch, we'd send the four byte header but with 0 - 1 =
>> 0xff in txbuf[3]. I'm pretty sure breakage would follow.
>>
>> Thus I'd like to have my patch in before the dp aux helpers use
>> msg->size == 0 for address only messages.
>>
>> Daniel, convinced yet?
>
> Indeed, I've been blinding. Acked for merging through radeon trees
> together with the other patches as a fixup for 3.15.

Christian,

Can you pull this into the radeon 3.15 branch as well?

Thanks,

Alex
Christian König April 8, 2014, 1:09 p.m. UTC | #6
Am 08.04.2014 15:04, schrieb Alex Deucher:
> On Tue, Apr 8, 2014 at 4:03 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Tue, Apr 8, 2014 at 8:58 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>>> Before the conversion to dp aux helpers there was a switch case [1] that
>>> ended up in msg_bytes = 3 for address only start/stop messages
>>> (MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
>>> but without my patch, we'd send the four byte header but with 0 - 1 =
>>> 0xff in txbuf[3]. I'm pretty sure breakage would follow.
>>>
>>> Thus I'd like to have my patch in before the dp aux helpers use
>>> msg->size == 0 for address only messages.
>>>
>>> Daniel, convinced yet?
>> Indeed, I've been blinding. Acked for merging through radeon trees
>> together with the other patches as a fixup for 3.15.
> Christian,
>
> Can you pull this into the radeon 3.15 branch as well?

Sure, directly before your other patches I would assume.

Going to send out the pull request to Dave this evening if nobody objects.

Christian.

>
> Thanks,
>
> Alex
Alex Deucher April 8, 2014, 1:11 p.m. UTC | #7
On Tue, Apr 8, 2014 at 9:09 AM, Christian König <deathsimple@vodafone.de> wrote:
> Am 08.04.2014 15:04, schrieb Alex Deucher:
>
>> On Tue, Apr 8, 2014 at 4:03 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>>
>>> On Tue, Apr 8, 2014 at 8:58 AM, Jani Nikula <jani.nikula@intel.com>
>>> wrote:
>>>>
>>>> Before the conversion to dp aux helpers there was a switch case [1] that
>>>> ended up in msg_bytes = 3 for address only start/stop messages
>>>> (MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
>>>> but without my patch, we'd send the four byte header but with 0 - 1 =
>>>> 0xff in txbuf[3]. I'm pretty sure breakage would follow.
>>>>
>>>> Thus I'd like to have my patch in before the dp aux helpers use
>>>> msg->size == 0 for address only messages.
>>>>
>>>> Daniel, convinced yet?
>>>
>>> Indeed, I've been blinding. Acked for merging through radeon trees
>>> together with the other patches as a fixup for 3.15.
>>
>> Christian,
>>
>> Can you pull this into the radeon 3.15 branch as well?
>
>
> Sure, directly before your other patches I would assume.

yes.

>
> Going to send out the pull request to Dave this evening if nobody objects.
>

Sounds good to me.

Thanks!

Alex

> Christian.
>
>>
>> Thanks,
>>
>> Alex
>
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e48d47ced57b..b435d07fbc08 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -575,7 +575,8 @@  out:
 	return ret;
 }
 
-#define HEADER_SIZE	4
+#define BARE_ADDRESS_SIZE	3
+#define HEADER_SIZE		(BARE_ADDRESS_SIZE + 1)
 static ssize_t
 intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
@@ -592,7 +593,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:
-		txsize = HEADER_SIZE + msg->size;
+		txsize = msg->size ? HEADER_SIZE + msg->size : BARE_ADDRESS_SIZE;
 		rxsize = 1;
 
 		if (WARN_ON(txsize > 20))
@@ -611,7 +612,7 @@  intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 
 	case DP_AUX_NATIVE_READ:
 	case DP_AUX_I2C_READ:
-		txsize = HEADER_SIZE;
+		txsize = msg->size ? HEADER_SIZE : BARE_ADDRESS_SIZE;
 		rxsize = msg->size + 1;
 
 		if (WARN_ON(rxsize > 20))