diff mbox

drm/i915: Warn about aux msg buffer vs. size mismatch

Message ID 1469717704-13020-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä July 28, 2016, 2:55 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If we have have a buffer, we should also have a size, and vice versa.
Let's check it both ways instead of just one.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Daniel Vetter July 28, 2016, 8:15 p.m. UTC | #1 
On Thu, Jul 28, 2016 at 05:55:04PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> If we have have a buffer, we should also have a size, and vice versa.
> Let's check it both ways instead of just one.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 21b04c3eda41..001f74fc0ce5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1041,10 +1041,10 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  		if (WARN_ON(txsize > 20))
>  			return -E2BIG;
>  
> +		WARN_ON(!msg->buffer != !msg->size);
> +
>  		if (msg->buffer)
>  			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
> -		else
> -			WARN_ON(msg->size);
>  
>  		ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
>  		if (ret > 0) {
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Aug. 2, 2016, 1:24 p.m. UTC | #2 
On Thu, Jul 28, 2016 at 03:20:52PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Warn about aux msg buffer vs. size mismatch
> URL   : https://patchwork.freedesktop.org/series/10356/
> State : failure
> 
> == Summary ==
> 
> Series 10356v1 drm/i915: Warn about aux msg buffer vs. size mismatch
> http://patchwork.freedesktop.org/api/1.0/series/10356/revisions/1/mbox
> 
> Test drv_module_reload_basic:
>                 skip       -> PASS       (ro-ivb-i7-3770)
>                 skip       -> PASS       (ro-skl3-i5-6260u)
> Test kms_cursor_legacy:
>         Subgroup basic-cursor-vs-flip-legacy:
>                 fail       -> PASS       (ro-ilk1-i5-650)
>         Subgroup basic-cursor-vs-flip-varying-size:
>                 pass       -> FAIL       (ro-ilk1-i5-650)

(kms_cursor_legacy:7977) DEBUG: Test requirement passed: target > 1
(kms_cursor_legacy:7977) DEBUG: Using a target of 32 cursor updates per half-vblank
(kms_cursor_legacy:7977) WARNING: page flip 0 was delayed, missed 1 frames
(kms_cursor_legacy:7977) CRITICAL: Test assertion failure function basic_cursor_vs_flip, file kms_cursor_legacy.c:670:
(kms_cursor_legacy:7977) CRITICAL: Failed assertion: vbl.sequence == vblank_start + 60
(kms_cursor_legacy:7977) CRITICAL: error: 11594 != 11593

https://bugs.freedesktop.org/show_bug.cgi?id=96701

>         Subgroup basic-flip-vs-cursor-legacy:
>                 fail       -> PASS       (fi-skl-i7-6700k)
>         Subgroup basic-flip-vs-cursor-varying-size:
>                 fail       -> PASS       (ro-snb-i7-2620M)
>                 pass       -> FAIL       (ro-bdw-i5-5250u)

(kms_cursor_legacy:8640) DEBUG: Test requirement passed: target > 1
(kms_cursor_legacy:8640) DEBUG: Using a target of 64 cursor updates per half-vblank
(kms_cursor_legacy:8640) CRITICAL: Test assertion failure function basic_flip_vs_cursor, file kms_cursor_legacy.c:514:
(kms_cursor_legacy:8640) CRITICAL: Failed assertion: get_vblank(display->drm_fd, pipe, 0) == vblank_start
(kms_cursor_legacy:8640) CRITICAL: error: 12134 != 12133

https://bugs.freedesktop.org/show_bug.cgi?id=97188

> fi-hsw-i7-4770k  total:239  pass:217  dwarn:0   dfail:0   fail:0   skip:22 
> fi-kbl-qkkr      total:239  pass:180  dwarn:29  dfail:0   fail:3   skip:27 
> fi-skl-i7-6700k  total:239  pass:208  dwarn:0   dfail:0   fail:3   skip:28 
> fi-snb-i7-2600   total:239  pass:197  dwarn:0   dfail:0   fail:0   skip:42 
> ro-bdw-i5-5250u  total:239  pass:218  dwarn:4   dfail:0   fail:1   skip:16 
> ro-bdw-i7-5600u  total:239  pass:206  dwarn:0   dfail:0   fail:1   skip:32 
> ro-bsw-n3050     total:239  pass:193  dwarn:0   dfail:0   fail:4   skip:42 
> ro-byt-n2820     total:239  pass:195  dwarn:0   dfail:0   fail:4   skip:40 
> ro-hsw-i7-4770r  total:239  pass:213  dwarn:0   dfail:0   fail:0   skip:26 
> ro-ilk1-i5-650   total:234  pass:172  dwarn:0   dfail:0   fail:2   skip:60 
> ro-ivb-i7-3770   total:239  pass:204  dwarn:0   dfail:0   fail:0   skip:35 
> ro-ivb2-i7-3770  total:239  pass:208  dwarn:0   dfail:0   fail:0   skip:31 
> ro-skl3-i5-6260u total:239  pass:221  dwarn:0   dfail:0   fail:4   skip:14 
> ro-snb-i7-2620M  total:239  pass:197  dwarn:0   dfail:0   fail:1   skip:41 
> fi-skl-i5-6260u failed to connect after reboot
> ro-bdw-i7-5557U failed to connect after reboot
> ro-ilk-i7-620lm failed to connect after reboot
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1632/
> 
> cb7629d drm-intel-nightly: 2016y-07m-28d-11h-02m-33s UTC integration manifest
> cc80144 drm/i915: Warn about aux msg buffer vs. size mismatch
Ville Syrjälä Aug. 2, 2016, 1:41 p.m. UTC | #3 
On Thu, Jul 28, 2016 at 10:15:35PM +0200, Daniel Vetter wrote:
> On Thu, Jul 28, 2016 at 05:55:04PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If we have have a buffer, we should also have a size, and vice versa.
> > Let's check it both ways instead of just one.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Pushed to dinq. Thanks for the review.

> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 21b04c3eda41..001f74fc0ce5 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1041,10 +1041,10 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> >  		if (WARN_ON(txsize > 20))
> >  			return -E2BIG;
> >  
> > +		WARN_ON(!msg->buffer != !msg->size);
> > +
> >  		if (msg->buffer)
> >  			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
> > -		else
> > -			WARN_ON(msg->size);
> >  
> >  		ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
> >  		if (ret > 0) {
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 21b04c3eda41..001f74fc0ce5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1041,10 +1041,10 @@  intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 		if (WARN_ON(txsize > 20))
 			return -E2BIG;
 
+		WARN_ON(!msg->buffer != !msg->size);
+
 		if (msg->buffer)
 			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
-		else
-			WARN_ON(msg->size);
 
 		ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
 		if (ret > 0) {