On Tue, Aug 30, 2022 at 11:27:27AM +0300, Jani Nikula wrote:
> On Mon, 29 Aug 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Turns out the MIPI sequence block version number and
> > new block size fields are considered part of the block
> > header and are not included in the reported new block size
> > field itself. Bump up the block size appropriately so that
> > we'll copy over the last five bytes of the block as well.
> >
> > For this particular machine those last five bytes included
> > parts of the GPIO op for the backlight on sequence, causing
> > the backlight no longer to turn back on:
> >
> > Sequence 6 - MIPI_SEQ_BACKLIGHT_ON
> > Delay: 20000 us
> > - GPIO index 0, number 0, set 0 (0x00)
> > + GPIO index 1, number 70, set 1 (0x01)
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: e163cfb4c96d ("drm/i915/bios: Make copies of VBT data blocks")
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6652
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_bios.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> > index 81d6cfbd2615..d493d04f4049 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> > @@ -479,6 +479,13 @@ init_bdb_block(struct drm_i915_private *i915,
> >
> > block_size = get_blocksize(block);
> >
> > + /*
> > + * Version number and new block size are considered
> > + * part of the header for MIPI sequenece block v3+.
> > + */
>
> Quoth Bspec, "This field specifies the size of this entire data
> structure excluding this header." Okay.
>
> > + if (section_id == BDB_MIPI_SEQUENCE && *(const u8 *)block >= 3)
> > + block_size += 5;
> > +
>
> Since we need to look at the header later, we can't just use data +
> 5. Okay.
>
> Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Thanks.
>
> I just don't get the whole sequence block u32 size thing, because the
> vbt header and bdb header still have u16, and they're supposed to cover
> the entire vbt and all bdb data blocks. And this is what we check
> anyway.
Yeah, very much snafu.
>
> > entry = kzalloc(struct_size(entry, data, max(min_size, block_size) + 3),
> > GFP_KERNEL);
> > if (!entry) {
>
> --
> Jani Nikula, Intel Open Source Graphics Center
--
Ville Syrjälä
Intel
