On Tue, Feb 10, 2026 at 11:23:02AM +0200, Jani Nikula wrote:
> On Fri, 06 Feb 2026, Samasth Norway Ananda <[email protected]>
> wrote:
> > When reading exactly 512 bytes with burst read enabled, the
> > extra_byte_added path breaks out of the inner do-while without
> > decrementing len. The outer while(len) then re-enters and gmbus_wait()
> > times out since all data has been delivered. Decrement len before the
> > break so the outer loop terminates correctly.
>
> Nice find, and the fix looks correct. How did you figure this out? Did
> you hit the issue?
>
> I wonder if the whole extra byte thing is a workaround for some old
> hardware that shouldn't be needed on modern hardware... Ville, thoughts?
I think it's still needed due to the weird way this is implemented
in the hardware. The byte counter rolls over at 256->1, so for that
to happen the programmed byte count must be >256 or else we'd reach
the target byte count before the rollover happens, in which case
the entire transfer would terminate already during the first 256
byte chunk.
>
> > Also fix a typo in a nearby comment ("generata" -> "generate").
>
> "Also" is a good hint that it should be a separate patch. ;)
>
> BR,
> Jani
>
> > Fixes: d5dc0f43f268 ("drm/i915/gmbus: Enable burst read")
> > Signed-off-by: Samasth Norway Ananda <[email protected]>
> > ---
> > drivers/gpu/drm/i915/display/intel_gmbus.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c
> > b/drivers/gpu/drm/i915/display/intel_gmbus.c
> > index 2caff677600c..5fb3fee34af4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> > +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> > @@ -496,8 +496,10 @@ gmbus_xfer_read_chunk(struct intel_display *display,
> >
> > val = intel_de_read_fw(display, GMBUS3(display));
> > do {
> > - if (extra_byte_added && len == 1)
> > + if (extra_byte_added && len == 1) {
> > + len--;
> > break;
> > + }
> >
> > *buf++ = val & 0xff;
> > val >>= 8;
> > @@ -693,7 +695,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct
> > i2c_msg *msgs, int num,
> > goto clear_err;
> > }
> >
> > - /* Generate a STOP condition on the bus. Note that gmbus can't generata
> > + /* Generate a STOP condition on the bus. Note that gmbus can't generate
> > * a STOP on the very first cycle. To simplify the code we
> > * unconditionally generate the STOP condition with an additional gmbus
> > * cycle. */
>
> --
> Jani Nikula, Intel
--
Ville Syrjälä
Intel
