On Wed, 19 Aug 2015, Mika Kahola <mika.kahola@xxxxxxxxx> wrote: > 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@xxxxxxxxx> wrote: >> > This reverts >> > >> > commit 047fe6e6db9161e69271f56daaafdaf2add023b1 >> > Author: David Weinehall <david.weinehall@xxxxxxxxxxxxxxx> >> > 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. That doesn't work because it breaks parse_sdvo_device_mapping(). I've fixed this now in drm-intel-next-fixes to be queued for v4.3, but I do not want to start patching up broken commits at v4.2-rc7 stage. As I've said, if Ville's commit 90e4f1592bb6 really does break stuff in v4.2 (*), David's commit 047fe6e6db91 does not fix it in v4.2 anyway. Thus I've applied the revert. It is the minimal fix. v4.2 will be fine. For v4.3 I'll need to make a backmerge from drm-intel-fixes to drm-intel-next-fixes, and we can move on to fixing v4.3. BR, Jani. (*) which I highly doubt in the first place; likely this was seen in -nightly and erroneously attributed as a regression on 90e4f1592bb6. > > 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@xxxxxxxx> >> > Cc: David Weinehall <david.weinehall@xxxxxxxxxxxxxxx> >> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> > >> > --- >> > >> > 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 >> > >> >> >> > > -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
