On Wed, 04 Feb 2026, Jonathan Cavitt <[email protected]> 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 <[email protected]>
> Signed-off-by: Jonathan Cavitt <[email protected]>

Reviewed-by: Jani Nikula <[email protected]>

> ---
>  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

Reply via email to