Re: [PATCH] drm/i915/gvt: Swap read and write checks

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 04 Feb 2026, Jonathan Cavitt <jonathan.cavitt@intel.com> wrote:
> The function intel_gvt_i2c_handle_aux_ch_write currently does not
> support the DP_AUX_I2C_WRITE operation.  Notably, we check if
> op & 0x1 == DP_AUX_I2C_WRITE (one), and if it does not, assert that
> op & 0x1 == DP_AUX_I2C_READ (zero).  This is unnecessary because if
> op & 0x1 != 1, then op & 0x1 == 0.  But beyond that, it probably makes
> more sense to check for the condition that is implemented, rather than
> check for the condition that is not.
>
> Swap the conditions.  We can also get rid of the unnecessary drm_WARN_ON
> while we're here.
>
> Suggested-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>

Thanks for the patch, pushed to drm-intel-next.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/gvt/edid.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/edid.c b/drivers/gpu/drm/i915/gvt/edid.c
> index 021afff1cd5d..ca5b54466a65 100644
> --- a/drivers/gpu/drm/i915/gvt/edid.c
> +++ b/drivers/gpu/drm/i915/gvt/edid.c
> @@ -535,16 +535,7 @@ void intel_gvt_i2c_handle_aux_ch_write(struct intel_vgpu *vgpu,
>  					i2c_edid->edid_available = true;
>  			}
>  		}
> -	} else if ((op & 0x1) == DP_AUX_I2C_WRITE) {
> -		/* TODO
> -		 * We only support EDID reading from I2C_over_AUX. And
> -		 * we do not expect the index mode to be used. Right now
> -		 * the WRITE operation is ignored. It is good enough to
> -		 * support the gfx driver to do EDID access.
> -		 */
> -	} else {
> -		if (drm_WARN_ON(&i915->drm, (op & 0x1) != DP_AUX_I2C_READ))
> -			return;
> +	} else if ((op & 0x1) == DP_AUX_I2C_READ) {
>  		if (drm_WARN_ON(&i915->drm, msg_length != 4))
>  			return;
>  		if (i2c_edid->edid_available && i2c_edid->target_selected) {
> @@ -553,6 +544,13 @@ void intel_gvt_i2c_handle_aux_ch_write(struct intel_vgpu *vgpu,
>  			aux_data_for_write = (val << 16);
>  		} else
>  			aux_data_for_write = (0xff << 16);
> +	} else {
> +		/* TODO
> +		 * We only support EDID reading from I2C_over_AUX. And
> +		 * we do not expect the index mode to be used. Right now
> +		 * the WRITE operation is ignored. It is good enough to
> +		 * support the gfx driver to do EDID access.
> +		 */
>  	}
>  	/* write the return value in AUX_CH_DATA reg which includes:
>  	 * ACK of I2C_WRITE

-- 
Jani Nikula, Intel