[bug report] drm/i915/gvt: vGPU display virtualization

[bug report] drm/i915/gvt: vGPU display virtualization

[Dan Carpenter]

Hello Zhi Wang,

The patch 04d348ae3f0a: "drm/i915/gvt: vGPU display virtualization"
from Apr 25, 2016, leads to the following static checker warning:

	drivers/gpu/drm/i915/gvt/edid.c:506 intel_gvt_i2c_handle_aux_ch_write()
	warn: odd binop '0x0 & 0xff'

drivers/gpu/drm/i915/gvt/edid.c
   501          /* write the return value in AUX_CH_DATA reg which includes:
   502           * ACK of I2C_WRITE
   503           * returned byte if it is READ
   504           */
   505  
   506          aux_data_for_write |= (GVT_AUX_I2C_REPLY_ACK & 0xff) << 24;

GVT_AUX_I2C_REPLY_ACK is 0 << 6.  Which is weird.  The whole line is a
no-op.

   507          vgpu_vreg(vgpu, offset + 4) = aux_data_for_write;
   508  }

regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igvt-g-dev] [bug report] drm/i915/gvt: vGPU display virtualization

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1011 bytes --]


On 2016.11.10 15:54:51 +0300, Dan Carpenter wrote:
> Hello Zhi Wang,
> 
> The patch 04d348ae3f0a: "drm/i915/gvt: vGPU display virtualization"
> from Apr 25, 2016, leads to the following static checker warning:
> 
> 	drivers/gpu/drm/i915/gvt/edid.c:506 intel_gvt_i2c_handle_aux_ch_write()
> 	warn: odd binop '0x0 & 0xff'
> 
> drivers/gpu/drm/i915/gvt/edid.c
>    501          /* write the return value in AUX_CH_DATA reg which includes:
>    502           * ACK of I2C_WRITE
>    503           * returned byte if it is READ
>    504           */
>    505  
>    506          aux_data_for_write |= (GVT_AUX_I2C_REPLY_ACK & 0xff) << 24;
> 
> GVT_AUX_I2C_REPLY_ACK is 0 << 6.  Which is weird.  The whole line is a
> no-op.

This is from DP spec for AUX i2c ack reply defined as 0 that we emulated
to reply. I think it's ok to keep as more explicitly to show reply command.

Thanks.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igvt-g-dev] [bug report] drm/i915/gvt: vGPU display virtualization

[Chris Wilson]

On Fri, Nov 11, 2016 at 04:08:58PM +0800, Zhenyu Wang wrote:
> 
> On 2016.11.10 15:54:51 +0300, Dan Carpenter wrote:
> > Hello Zhi Wang,
> > 
> > The patch 04d348ae3f0a: "drm/i915/gvt: vGPU display virtualization"
> > from Apr 25, 2016, leads to the following static checker warning:
> > 
> > 	drivers/gpu/drm/i915/gvt/edid.c:506 intel_gvt_i2c_handle_aux_ch_write()
> > 	warn: odd binop '0x0 & 0xff'
> > 
> > drivers/gpu/drm/i915/gvt/edid.c
> >    501          /* write the return value in AUX_CH_DATA reg which includes:
> >    502           * ACK of I2C_WRITE
> >    503           * returned byte if it is READ
> >    504           */
> >    505  
> >    506          aux_data_for_write |= (GVT_AUX_I2C_REPLY_ACK & 0xff) << 24;
> > 
> > GVT_AUX_I2C_REPLY_ACK is 0 << 6.  Which is weird.  The whole line is a
> > no-op.
> 
> This is from DP spec for AUX i2c ack reply defined as 0 that we emulated
> to reply. I think it's ok to keep as more explicitly to show reply command.

The line is confusing though: you imply that REPLY_ACK is > 256 and that
you only want the low 8bits stuffed into the high 8bits of the dword.
Sounds very weird for the definition, and you wonder whether it is safe.

	aux_data_for_write |= GVT_AUX_I2C_REPLY_ACK << 24;

is still a no-op but should not affend too many sensibilities.
-Chirs

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igvt-g-dev] [bug report] drm/i915/gvt: vGPU display virtualization

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1599 bytes --]

On 2016.11.11 08:28:28 +0000, Chris Wilson wrote:
> On Fri, Nov 11, 2016 at 04:08:58PM +0800, Zhenyu Wang wrote:
> > 
> > On 2016.11.10 15:54:51 +0300, Dan Carpenter wrote:
> > > Hello Zhi Wang,
> > > 
> > > The patch 04d348ae3f0a: "drm/i915/gvt: vGPU display virtualization"
> > > from Apr 25, 2016, leads to the following static checker warning:
> > > 
> > > 	drivers/gpu/drm/i915/gvt/edid.c:506 intel_gvt_i2c_handle_aux_ch_write()
> > > 	warn: odd binop '0x0 & 0xff'
> > > 
> > > drivers/gpu/drm/i915/gvt/edid.c
> > >    501          /* write the return value in AUX_CH_DATA reg which includes:
> > >    502           * ACK of I2C_WRITE
> > >    503           * returned byte if it is READ
> > >    504           */
> > >    505  
> > >    506          aux_data_for_write |= (GVT_AUX_I2C_REPLY_ACK & 0xff) << 24;
> > > 
> > > GVT_AUX_I2C_REPLY_ACK is 0 << 6.  Which is weird.  The whole line is a
> > > no-op.
> > 
> > This is from DP spec for AUX i2c ack reply defined as 0 that we emulated
> > to reply. I think it's ok to keep as more explicitly to show reply command.
> 
> The line is confusing though: you imply that REPLY_ACK is > 256 and that
> you only want the low 8bits stuffed into the high 8bits of the dword.
> Sounds very weird for the definition, and you wonder whether it is safe.
> 
> 	aux_data_for_write |= GVT_AUX_I2C_REPLY_ACK << 24;
> 
> is still a no-op but should not affend too many sensibilities.

Will change like that. Thanks.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx