Re: [PATCH for v4.2] Revert "drm/i915: Allow parsing of variable size child device entries from VBT"

[Mika Kahola <mika.kahola@intel.com>]

On Tue, 2015-08-18 at 09:58 -0700, Daniel Vetter wrote:
> On Tue, Aug 18, 2015 at 2:33 AM, Jani Nikula <jani.nikula@intel.com> wrote:
> > This reverts
> >
> > commit 047fe6e6db9161e69271f56daaafdaf2add023b1
> > Author: David Weinehall <david.weinehall@linux.intel.com>
> > Date:   Tue Aug 4 16:55:52 2015 +0300
> >
> >     drm/i915: Allow parsing of variable size child device entries from VBT
> >
> > That commit is not valid for v4.2, however it will be valid for v4.3. It
> > was simply queued too early.
> 
> Nope, this patch from David also incidentally fixes a regression from
> 90e4f1592bb6e82f6690f0e05a8aadc which is why I merged it to. If you
> want to revert this, you need to revert more (and with that also break
> BSW).
> 
> Isn't there a more minimal fix we can do for 4.2?
I proposed a very minimal solution just to increase the size of the raw
buffer to the maximum of 38 bytes so we could later on parse the correct
info.

May not be relevant anymore

https://bugs.freedesktop.org/show_bug.cgi?id=91269

-Mika-

> -Daniel
> 
> >
> > The referenced regressing commit is just fine until the size of struct
> > common_child_dev_config changes, and that won't happen until
> > v4.3. Indeed, the expected size checks here rely on the increased size
> > of the struct, breaking new platforms.
> >
> > Fixes: 047fe6e6db91 ("drm/i915: Allow parsing of variable size child device entries from VBT")
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >
> > ---
> >
> > There was another patch from David increasing the size of the struct
> > [1], but that then regresses sdvo mapping parsing. It's the simplest to
> > just revert and fix this up properly for v4.3.
> >
> > [1] http://mid.gmane.org/20150813131415.GO6150@boom
> > ---
> >  drivers/gpu/drm/i915/intel_bios.c | 27 ++++-----------------------
> >  1 file changed, 4 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index 3dcd59e694db..198fc3c3291b 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -1075,34 +1075,15 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> >         const union child_device_config *p_child;
> >         union child_device_config *child_dev_ptr;
> >         int i, child_device_num, count;
> > -       u8 expected_size;
> > -       u16 block_size;
> > +       u16     block_size;
> >
> >         p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
> >         if (!p_defs) {
> >                 DRM_DEBUG_KMS("No general definition block is found, no devices defined.\n");
> >                 return;
> >         }
> > -       if (bdb->version < 195) {
> > -               expected_size = 33;
> > -       } else if (bdb->version == 195) {
> > -               expected_size = 37;
> > -       } else if (bdb->version <= 197) {
> > -               expected_size = 38;
> > -       } else {
> > -               expected_size = 38;
> > -               DRM_DEBUG_DRIVER("Expected child_device_config size for BDB version %u not known; assuming %u\n",
> > -                                expected_size, bdb->version);
> > -       }
> > -
> > -       if (expected_size > sizeof(*p_child)) {
> > -               DRM_ERROR("child_device_config cannot fit in p_child\n");
> > -               return;
> > -       }
> > -
> > -       if (p_defs->child_dev_size != expected_size) {
> > -               DRM_ERROR("Size mismatch; child_device_config size=%u (expected %u); bdb->version: %u\n",
> > -                         p_defs->child_dev_size, expected_size, bdb->version);
> > +       if (p_defs->child_dev_size < sizeof(*p_child)) {
> > +               DRM_ERROR("General definiton block child device size is too small.\n");
> >                 return;
> >         }
> >         /* get the block size of general definitions */
> > @@ -1149,7 +1130,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> >
> >                 child_dev_ptr = dev_priv->vbt.child_dev + count;
> >                 count++;
> > -               memcpy(child_dev_ptr, p_child, p_defs->child_dev_size);
> > +               memcpy(child_dev_ptr, p_child, sizeof(*p_child));
> >         }
> >         return;
> >  }
> > --
> > 2.1.4
> >
> 
> 
> 


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