[PATCH 1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()

[PATCH 1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're supposed to examine msgs[i] and msgs[i+1] to see if they
form a pair suitable for an indexed transfer. But in reality
we're examining msgs[0] and msgs[1]. Fix this.

Cc: stable@vger.kernel.org
Cc: Daniel Kurtz <djkurtz@chromium.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sean Paul <seanpaul@chromium.org>
Fixes: 56f9eac05489 ("drm/i915/intel_i2c: use INDEX cycles for i2c read transactions")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index eb5827110d8f..165375cbef2f 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -484,7 +484,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
 
 	for (; i < num; i += inc) {
 		inc = 1;
-		if (gmbus_is_index_read(msgs, i, num)) {
+		if (gmbus_is_index_read(&msgs[i], i, num)) {
 			ret = gmbus_xfer_index_read(dev_priv, &msgs[i]);
 			inc = 2; /* an index read is two msgs */
 		} else if (msgs[i].flags & I2C_M_RD) {
-- 
2.13.6

[PATCH 2/3] drm/i915: Don't try indexed reads to alternate slave addresses

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We can only specify the one slave address to indexed reads/writes.
Make sure the messages we check are destined to the same slave
address before deciding to do an indexed transfer.

Cc: stable@vger.kernel.org
Cc: Daniel Kurtz <djkurtz@chromium.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sean Paul <seanpaul@chromium.org>
Fixes: 56f9eac05489 ("drm/i915/intel_i2c: use INDEX cycles for i2c read transactions")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_i2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 165375cbef2f..4b4528d7010f 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -438,6 +438,7 @@ static bool
 gmbus_is_index_read(struct i2c_msg *msgs, int i, int num)
 {
 	return (i + 1 < num &&
+		msgs[i].addr == msgs[i + 1].addr &&
 		!(msgs[i].flags & I2C_M_RD) && msgs[i].len <= 2 &&
 		(msgs[i + 1].flags & I2C_M_RD));
 }
-- 
2.13.6

Re: [PATCH 2/3] drm/i915: Don't try indexed reads to alternate slave addresses

[Chris Wilson]

Quoting Ville Syrjala (2017-11-23 19:41:56)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We can only specify the one slave address to indexed reads/writes.
> Make sure the messages we check are destined to the same slave
> address before deciding to do an indexed transfer.
> 
> Cc: stable@vger.kernel.org
> Cc: Daniel Kurtz <djkurtz@chromium.org>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sean Paul <seanpaul@chromium.org>
> Fixes: 56f9eac05489 ("drm/i915/intel_i2c: use INDEX cycles for i2c read transactions")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

[PATCH 3/3] drm/i915: Prevent zero length "index" write

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The hardware always writes one or two bytes in the index portion of
an indexed transfer. Make sure the message we send as the index
doesn't have a zero length.

Cc: stable@vger.kernel.org
Cc: Daniel Kurtz <djkurtz@chromium.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sean Paul <seanpaul@chromium.org>
Fixes: 56f9eac05489 ("drm/i915/intel_i2c: use INDEX cycles for i2c read transactions")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_i2c.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index 4b4528d7010f..8c6b26b5c3fb 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -439,7 +439,8 @@ gmbus_is_index_read(struct i2c_msg *msgs, int i, int num)
 {
 	return (i + 1 < num &&
 		msgs[i].addr == msgs[i + 1].addr &&
-		!(msgs[i].flags & I2C_M_RD) && msgs[i].len <= 2 &&
+		!(msgs[i].flags & I2C_M_RD) &&
+		(msgs[i].len == 1 || msgs[i].len == 2) &&
 		(msgs[i + 1].flags & I2C_M_RD));
 }
 
-- 
2.13.6

Re: [PATCH 3/3] drm/i915: Prevent zero length "index" write

[Chris Wilson]

Quoting Ville Syrjala (2017-11-23 19:41:57)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The hardware always writes one or two bytes in the index portion of
> an indexed transfer. Make sure the message we send as the index
> doesn't have a zero length.
> 
> Cc: stable@vger.kernel.org
> Cc: Daniel Kurtz <djkurtz@chromium.org>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sean Paul <seanpaul@chromium.org>
> Fixes: 56f9eac05489 ("drm/i915/intel_i2c: use INDEX cycles for i2c read transactions")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()
URL   : https://patchwork.freedesktop.org/series/34329/
State : success

== Summary ==

Series 34329v1 series starting with [1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()
https://patchwork.freedesktop.org/api/1.0/series/34329/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-cfl-s2)

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:446s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:458s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:437s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:550s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:279s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:508s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:513s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:503s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:507s
fi-cfl-s2        total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:617s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:434s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:271s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:433s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:441s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:430s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:467s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:578s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:460s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:544s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:573s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:527s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:503s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:460s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:561s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:424s
Blacklisted hosts:
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:543s
fi-glk-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:500s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:483s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:528s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:533s

b407e5f38397c0c22b5056a1664753287993b152 drm-tip: 2017y-11m-23d-16h-14m-59s UTC integration manifest
cdb66eef313c drm/i915: Prevent zero length "index" write
de4e63cd1642 drm/i915: Don't try indexed reads to alternate slave addresses
eddcaf4bb19a drm/i915: Pass the correct msgs to gmbus_is_index_read()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7268/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()

[Chris Wilson]

Quoting Ville Syrjala (2017-11-23 19:41:55)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're supposed to examine msgs[i] and msgs[i+1] to see if they
> form a pair suitable for an indexed transfer. But in reality
> we're examining msgs[0] and msgs[1]. Fix this.
> 
> Cc: stable@vger.kernel.org
> Cc: Daniel Kurtz <djkurtz@chromium.org>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sean Paul <seanpaul@chromium.org>
> Fixes: 56f9eac05489 ("drm/i915/intel_i2c: use INDEX cycles for i2c read transactions")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index eb5827110d8f..165375cbef2f 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -484,7 +484,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>  
>         for (; i < num; i += inc) {
>                 inc = 1;
> -               if (gmbus_is_index_read(msgs, i, num)) {
> +               if (gmbus_is_index_read(&msgs[i], i, num)) {

i is passed to gmbus_is_index_read() and used as an index into msgs. So
this should be accounted for right?
-Chris

Re: [PATCH 1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()

[Ville Syrjälä]

On Thu, Nov 23, 2017 at 08:50:41PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-23 19:41:55)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We're supposed to examine msgs[i] and msgs[i+1] to see if they
> > form a pair suitable for an indexed transfer. But in reality
> > we're examining msgs[0] and msgs[1]. Fix this.
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Daniel Kurtz <djkurtz@chromium.org>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Fixes: 56f9eac05489 ("drm/i915/intel_i2c: use INDEX cycles for i2c read transactions")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_i2c.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> > index eb5827110d8f..165375cbef2f 100644
> > --- a/drivers/gpu/drm/i915/intel_i2c.c
> > +++ b/drivers/gpu/drm/i915/intel_i2c.c
> > @@ -484,7 +484,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
> >  
> >         for (; i < num; i += inc) {
> >                 inc = 1;
> > -               if (gmbus_is_index_read(msgs, i, num)) {
> > +               if (gmbus_is_index_read(&msgs[i], i, num)) {
> 
> i is passed to gmbus_is_index_read() and used as an index into msgs. So
> this should be accounted for right?

Doh. Yep, this patch is nonsense.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()

[Ville Syrjälä]

On Fri, Nov 24, 2017 at 02:55:28PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 23, 2017 at 08:50:41PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjala (2017-11-23 19:41:55)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We're supposed to examine msgs[i] and msgs[i+1] to see if they
> > > form a pair suitable for an indexed transfer. But in reality
> > > we're examining msgs[0] and msgs[1]. Fix this.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Cc: Daniel Kurtz <djkurtz@chromium.org>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: Sean Paul <seanpaul@chromium.org>
> > > Fixes: 56f9eac05489 ("drm/i915/intel_i2c: use INDEX cycles for i2c read transactions")
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_i2c.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> > > index eb5827110d8f..165375cbef2f 100644
> > > --- a/drivers/gpu/drm/i915/intel_i2c.c
> > > +++ b/drivers/gpu/drm/i915/intel_i2c.c
> > > @@ -484,7 +484,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
> > >  
> > >         for (; i < num; i += inc) {
> > >                 inc = 1;
> > > -               if (gmbus_is_index_read(msgs, i, num)) {
> > > +               if (gmbus_is_index_read(&msgs[i], i, num)) {
> > 
> > i is passed to gmbus_is_index_read() and used as an index into msgs. So
> > this should be accounted for right?
> 
> Doh. Yep, this patch is nonsense.

The two other patches pushed to dinq. Thanks catching my mistake with
this one.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Pass the correct msgs to gmbus_is_index_read()
URL   : https://patchwork.freedesktop.org/series/34329/
State : success

== Summary ==

Test perf:
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test drv_selftest:
        Subgroup mock_sanitycheck:
                pass       -> DMESG-WARN (shard-snb) fdo#103717
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                pass       -> FAIL       (shard-snb) fdo#101623

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#103717 https://bugs.freedesktop.org/show_bug.cgi?id=103717
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623

shard-hsw        total:2667 pass:1536 dwarn:1   dfail:0   fail:9   skip:1121 time:9530s
shard-snb        total:2667 pass:1311 dwarn:2   dfail:0   fail:13  skip:1341 time:8137s
Blacklisted hosts:
shard-apl        total:2645 pass:1663 dwarn:1   dfail:0   fail:26  skip:954 time:13147s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7268/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx