From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: Fix not finding the VBT when it overlaps with OPREGION_ASLE_EXT Date: Mon, 23 Jan 2017 12:36:01 +0200 Message-ID: <877f5m83n2.fsf@intel.com> References: <20161225101928.7618-1-hdegoede@redhat.com> <87pokd4p0k.fsf@intel.com> <067c9e05-a9a5-2432-22a8-bfd49ae706ff@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <067c9e05-a9a5-2432-22a8-bfd49ae706ff@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Hans de Goede , Daniel Vetter , Ville =?utf-8?B?U3lyasOkbMOk?= Cc: intel-gfx , stable@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCAyMCBKYW4gMjAxNywgSGFucyBkZSBHb2VkZSA8aGRlZ29lZGVAcmVkaGF0LmNvbT4g d3JvdGU6Cj4gSGksCj4KPiBPbiAzMS0xMi0xNiAxNzowMCwgSGFucyBkZSBHb2VkZSB3cm90ZToK Pj4gSGksCj4+Cj4+IE9uIDI3LTEyLTE2IDExOjU4LCBKYW5pIE5pa3VsYSB3cm90ZToKPj4+IE9u IFN1biwgMjUgRGVjIDIwMTYsIEhhbnMgZGUgR29lZGUgPGhkZWdvZWRlQHJlZGhhdC5jb20+IHdy b3RlOgo+Pj4+IElmIHRoZXJlIGlzIG5vIE9QUkVHSU9OX0FTTEVfRVhUIHRoZW4gYSBWQlQgc3Rv cmVkIGluIG1haWxib3ggIzQgbWF5Cj4+Pj4gdXNlIHRoZSBBU0xFX0VYVCBwYXJ0cyBvZiB0aGUg b3ByZWdpb24uIEFkanVzdCB0aGUgdmJ0X3NpemUgY2FsY3VsYXRpb24KPj4+PiBmb3IgYSB2YnQg aW4gbWFpbGJveCAjNCBmb3IgdGhpcy4KPj4+Pgo+Pj4+IFRoaXMgZml4ZXMgdGhlIGRyaXZlciBu b3QgZmluZGluZyB0aGUgVkJUIG9uIGEganVtcGVyIGV6cGFkIG1pbmkzCj4+Pj4gY2hlcnJ5dHJh aWwgdGFibGV0Lgo+Pj4KPj4+IFRoYW5rcyBmb3IgdGhlIHBhdGNoLiBJIHRoaW5rIHlvdSdyZSBv bnRvIHNvbWV0aGluZywgYnV0IEkgZG9uJ3QgdGhpbmsKPj4+IHRoZSBwYXRjaCBpcyBxdWl0ZSBj b3JyZWN0LiBUaGF0IHNhaWQsIEknbSBub3Qgc3VyZSBteXNlbGYgeWV0IHdoYXQKPj4+IHdvdWxk IGJlLiA7KQo+Pj4KPj4+IFdpdGhvdXQgdGhlIGNoYW5nZSwgZG9lcyBpbnRlbF9iaW9zX2lzX3Zh bGlkX3ZidCgpIHJldHVybiB0cnVlIGFueXdheT8KPj4KPj4gTm8uCj4+Cj4+PiBJLmUuIGRvIHlv dSBnZXQgIkZvdW5kIHZhbGlkIFZCVCBpbiBBQ1BJIE9wUmVnaW9uIChNYWlsYm94ICM0KVxuIiBp bgo+Pj4gbG9nPwo+Pgo+PiBOby4KPj4KPj4+IElmIG5vdCwgd2hpY2ggb2YgdGhlIGRlYnVnIG1l c3NhZ2VzIGluIGludGVsX2Jpb3NfaXNfdmFsaWRfdmJ0KCkgZG8KPj4+IHlvdSBnZXQ/Cj4+Cj4+ IEkgZ2V0ICJCREIgaW5jb21wbGV0ZSIsIHdoaWNoIGlzIHdoeSBJIHdyb3RlIHRoaXMgcGF0Y2gg YW5kIGJlbGlldmUKPj4gdGhpcyBwYXRjaCBpcyB0aGUgcmlnaHQgc29sdXRpb24uIFdpdGggdGhp cyBwYXRjaCBldmVyeXRoaW5nIHdvcmtzLAo+Pgo+Pj4gSW4gdGhlIGxhdHRlciBjYXNlLCBJIHN1 c3BlY3QgeW91J2xsIGVuZCB1cCB3aXRoIGZhaWx1cmUgaW4gaW50ZWxfYmlvcy5jCj4+PiB3aXRo IGVpdGhlciAiTm8gTUlQSSBjb25maWcgQkRCIGZvdW5kIiBvciAiTm8gTUlQSSBTZXF1ZW5jZSBm b3VuZCwKPj4+IHBhcnNpbmcgY29tcGxldGVcbiIuCj4+Cj4+IEkgZG9uJ3QgcmVtZW1iZXIgdGhl IGV4YWN0IGVycm9yLCBvdGhlciB0aGVuIGdldHRpbmcgdGhlCj4+ICJCREIgaW5jb21wbGV0ZSIg ZXJyb3IsIGFuZCB0aGUgaTkxNSBkcml2ZXIgbm90IGxpc3RpbmcgdGhlIERTSSBjb25uZWN0b3IK Pj4gdW5kZXIgL3N5cy9jbGFzcyBkcm0uCj4+Cj4+IFdoYXQgbWFrZXMgeW91IHNheTogImJ1dCBJ IGRvbid0IHRoaW5rIHRoZSBwYXRjaCBpcyBxdWl0ZSBjb3JyZWN0IiB3aHkKPj4gc2hvdWxkIHRo ZSBjb2RlIHN0aWxsIGtlZXAgdGhlIE9QUkVHSU9OX0FTTEVfRVhUIHN0YXJ0IGFzIGVuZCBvZiB0 aGUKPj4gbWFpbGJveCAjNCB2YnQgaWYgdGhlcmUgaXMgdGhlIEFTTEUgZXh0ZW5zdGlvbiBpcyBu b3QgdXNlZCA/Cj4KPiBQaW5nLCBhbnkgcHJvZ3Jlc3Mgb24gdGhpcyA/IEkgc3RpbGwgYmVsaWV2 ZSBteSBvcmlnaW5hbCBwYXRjaCBpcwo+IGNvcnJlY3QuIEVpdGhlcndheSBJIHdvdWxkIGxpa2Ug dG8gc2VlIGEgZml4IGZvciB0aGlzLCBiZSBpdCB0aGlzIGZpeAo+IG9yIHNvbWV0aGluZyBlbHNl LCBhcyBhIGZpeCBpcyBuZWNlc3NhcnkgdG8gZ2V0IHRoZSBMQ0QgcGFuZWwgdG8gd29yawo+IG9u IGp1bXBlciBlenBhZCBtaW5pMyB0YWJsZXRzLgoKUGxlYXNlIHNlbmQgbWUgL3N5cy9rZXJuZWwv ZGVidWcvZHJpLzAvaTkxNV9vcHJlZ2lvbiBvbiB0aGF0IG1hY2hpbmUuCgpQZXJ1c2luZyB0aGUg b3ByZWdpb24gc3BlYyAod2hpY2ggSSByZWdyZXQgSSBjYW4ndCBzaGFyZSB3aXRoIHlvdSksIEkK Zm91bmQgdGhpczoKCiogT24gbWFpbGJveGVzIGluIGdlbmVyYWwsICJNYWlsLWJveCBsb2NhdGlv bnMgYXJlIGZpeGVkIGFuZCBzaG91bGQKICBhbHdheXMgYmUgYWxsb2NhdGVkIGlycmVzcGVjdGl2 ZSBvZiB0aGUgc3VwcG9ydCBmb3IgYSBnaXZlbiBtYWlsIGJveAogIGlzIGF2YWlsYWJsZSBvciBu b3QuIgoKKiBPbiBvcHJlZ2lvbi0+YXNsZS0+cnZkYSwgIlRoaXMgaXMgbWFpbmx5IHVzZWQgd2hl biBWQlQgc2l6ZSBleGNlZWRzCiAgNktCIGFuZCBjYW4ndCBiZSBzdG9yZWQgaW4gTWFpbGJveDQu IiBJdCBpc24ndCBjbGVhciB0byBtZSB3aGV0aGVyCiAgLT5ydmRhIHdhcyB1c2VkIG9yIG5vdC4g VGhlIG9wcmVnaW9uIGR1bXAgc2hvdWxkIHNoZWQgbGlnaHQgb24KICB0aGlzLiBZb3UgY2FuIG9m IGNvdXJzZSBjaGVjayB0aGF0IGJ5IGFkZGluZyBkZWJ1ZyBwcmludHMgaW4gdGhlIGNvZGUKICB0 b28uCgoqIE9uIG1haWxib3ggNCAodGhlIFZCVCksICJIb2xkcyBhIG1heGltdW0gb2YgNktCIHNp emVkIFJhdyBWQlQgZGF0YQogIChub3QgVkJJT1MgaW1hZ2UpIGZyb20gVkJJT1MgaW1hZ2UuIgoK Q2xlYXJseSB0aGUgcGF0Y2ggaXMgYWdhaW5zdCB0aGUgc3BlYy4gTGV0J3Mgc2VlIGlmIHRoZSBv cHJlZ2lvbiB5b3UKaGF2ZSB0aGVyZSBpcyBhZ2FpbnN0IHRoZSBzcGVjIHRvbywgYW5kIHByb2Nl ZWQgZnJvbSB0aGVyZS4uLgoKQlIsCkphbmkuCgoKPgo+IFJlZ2FyZHMsCj4KPiBIYW5zCj4KPgo+ Cj4+Pj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcKPj4+PiBTaWduZWQtb2ZmLWJ5OiBIYW5z IGRlIEdvZWRlIDxoZGVnb2VkZUByZWRoYXQuY29tPgo+Pj4+IC0tLQo+Pj4+IE5vdGUgZXZlbiB3 aXRoIHRoaXMgZml4ZWQgdGhlIHBhbmVsIHN0aWxsIGRvZXMgbm90IHdvcmsgd2l0aCA0LjksCj4+ Pj4gYnV0IGl0IGRvZXMgd2l0aCBkcm0taW50ZWwtbmV4dC1xdWV1ZWQgOikgSSBiZWxpZXZlIHRo ZSBtaXNzaW5nIGJpdCBpbgo+Pj4+IDQuOSBpcyB0aGUgImRybS85MTU6IFBhcnNpbmcgdGhlIG1p c3NlZCBvdXQgRFREIGZpZWxkcyBmcm9tIHRoZSBWQlQiCj4+Pj4gY29tbWl0LCBidXQgSSd2ZSBu b3QgdmVyaWZpZWQgdGhpcy4KPj4+PiAtLS0KPj4+PiAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50 ZWxfb3ByZWdpb24uYyB8IDQgKysrLQo+Pj4+ICAxIGZpbGUgY2hhbmdlZCwgMyBpbnNlcnRpb25z KCspLCAxIGRlbGV0aW9uKC0pCj4+Pj4KPj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJt L2k5MTUvaW50ZWxfb3ByZWdpb24uYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX29wcmVn aW9uLmMKPj4+PiBpbmRleCBmNDQyOWY2Li5lZmYzNWFlIDEwMDY0NAo+Pj4+IC0tLSBhL2RyaXZl cnMvZ3B1L2RybS9pOTE1L2ludGVsX29wcmVnaW9uLmMKPj4+PiArKysgYi9kcml2ZXJzL2dwdS9k cm0vaTkxNS9pbnRlbF9vcHJlZ2lvbi5jCj4+Pj4gQEAgLTk4Miw3ICs5ODIsOSBAQCBpbnQgaW50 ZWxfb3ByZWdpb25fc2V0dXAoc3RydWN0IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2KQo+Pj4+ ICAgICAgICAgICAgICBvcHJlZ2lvbi0+dmJ0X3NpemUgPSB2YnRfc2l6ZTsKPj4+PiAgICAgICAg ICB9IGVsc2Ugewo+Pj4+ICAgICAgICAgICAgICB2YnQgPSBiYXNlICsgT1BSRUdJT05fVkJUX09G RlNFVDsKPj4+PiAtICAgICAgICAgICAgdmJ0X3NpemUgPSBPUFJFR0lPTl9BU0xFX0VYVF9PRkZT RVQgLSBPUFJFR0lPTl9WQlRfT0ZGU0VUOwo+Pj4+ICsgICAgICAgICAgICB2YnRfc2l6ZSA9ICht Ym94ZXMgJiBNQk9YX0FTTEVfRVhUKSA/Cj4+Pj4gKyAgICAgICAgICAgICAgICBPUFJFR0lPTl9B U0xFX0VYVF9PRkZTRVQgOiBPUFJFR0lPTl9TSVpFOwo+Pj4+ICsgICAgICAgICAgICB2YnRfc2l6 ZSAtPSBPUFJFR0lPTl9WQlRfT0ZGU0VUOwo+Pj4+ICAgICAgICAgICAgICBpZiAoaW50ZWxfYmlv c19pc192YWxpZF92YnQodmJ0LCB2YnRfc2l6ZSkpIHsKPj4+PiAgICAgICAgICAgICAgICAgIERS TV9ERUJVR19LTVMoIkZvdW5kIHZhbGlkIFZCVCBpbiBBQ1BJIE9wUmVnaW9uIChNYWlsYm94ICM0 KVxuIik7Cj4+Pj4gICAgICAgICAgICAgICAgICBvcHJlZ2lvbi0+dmJ0ID0gdmJ0Owo+Pj4KCi0t IApKYW5pIE5pa3VsYSwgSW50ZWwgT3BlbiBTb3VyY2UgVGVjaG5vbG9neSBDZW50ZXIKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxp bmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJl ZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com ([192.55.52.120]:46529 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750714AbdAWKjH (ORCPT ); Mon, 23 Jan 2017 05:39:07 -0500 From: Jani Nikula To: Hans de Goede , Daniel Vetter , Ville =?utf-8?B?U3lyasOkbMOk?= Cc: intel-gfx , dri-devel@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [PATCH] drm/i915: Fix not finding the VBT when it overlaps with OPREGION_ASLE_EXT In-Reply-To: <067c9e05-a9a5-2432-22a8-bfd49ae706ff@redhat.com> References: <20161225101928.7618-1-hdegoede@redhat.com> <87pokd4p0k.fsf@intel.com> <067c9e05-a9a5-2432-22a8-bfd49ae706ff@redhat.com> Date: Mon, 23 Jan 2017 12:36:01 +0200 Message-ID: <877f5m83n2.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: stable-owner@vger.kernel.org List-ID: On Fri, 20 Jan 2017, Hans de Goede wrote: > Hi, > > On 31-12-16 17:00, Hans de Goede wrote: >> Hi, >> >> On 27-12-16 11:58, Jani Nikula wrote: >>> On Sun, 25 Dec 2016, Hans de Goede wrote: >>>> If there is no OPREGION_ASLE_EXT then a VBT stored in mailbox #4 may >>>> use the ASLE_EXT parts of the opregion. Adjust the vbt_size calculation >>>> for a vbt in mailbox #4 for this. >>>> >>>> This fixes the driver not finding the VBT on a jumper ezpad mini3 >>>> cherrytrail tablet. >>> >>> Thanks for the patch. I think you're onto something, but I don't think >>> the patch is quite correct. That said, I'm not sure myself yet what >>> would be. ;) >>> >>> Without the change, does intel_bios_is_valid_vbt() return true anyway? >> >> No. >> >>> I.e. do you get "Found valid VBT in ACPI OpRegion (Mailbox #4)\n" in >>> log? >> >> No. >> >>> If not, which of the debug messages in intel_bios_is_valid_vbt() do >>> you get? >> >> I get "BDB incomplete", which is why I wrote this patch and believe >> this patch is the right solution. With this patch everything works, >> >>> In the latter case, I suspect you'll end up with failure in intel_bios.c >>> with either "No MIPI config BDB found" or "No MIPI Sequence found, >>> parsing complete\n". >> >> I don't remember the exact error, other then getting the >> "BDB incomplete" error, and the i915 driver not listing the DSI connector >> under /sys/class drm. >> >> What makes you say: "but I don't think the patch is quite correct" why >> should the code still keep the OPREGION_ASLE_EXT start as end of the >> mailbox #4 vbt if there is the ASLE extenstion is not used ? > > Ping, any progress on this ? I still believe my original patch is > correct. Eitherway I would like to see a fix for this, be it this fix > or something else, as a fix is necessary to get the LCD panel to work > on jumper ezpad mini3 tablets. Please send me /sys/kernel/debug/dri/0/i915_opregion on that machine. Perusing the opregion spec (which I regret I can't share with you), I found this: * On mailboxes in general, "Mail-box locations are fixed and should always be allocated irrespective of the support for a given mail box is available or not." * On opregion->asle->rvda, "This is mainly used when VBT size exceeds 6KB and can't be stored in Mailbox4." It isn't clear to me whether ->rvda was used or not. The opregion dump should shed light on this. You can of course check that by adding debug prints in the code too. * On mailbox 4 (the VBT), "Holds a maximum of 6KB sized Raw VBT data (not VBIOS image) from VBIOS image." Clearly the patch is against the spec. Let's see if the opregion you have there is against the spec too, and proceed from there... BR, Jani. > > Regards, > > Hans > > > >>>> Cc: stable@vger.kernel.org >>>> Signed-off-by: Hans de Goede >>>> --- >>>> Note even with this fixed the panel still does not work with 4.9, >>>> but it does with drm-intel-next-queued :) I believe the missing bit in >>>> 4.9 is the "drm/915: Parsing the missed out DTD fields from the VBT" >>>> commit, but I've not verified this. >>>> --- >>>> drivers/gpu/drm/i915/intel_opregion.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c >>>> index f4429f6..eff35ae 100644 >>>> --- a/drivers/gpu/drm/i915/intel_opregion.c >>>> +++ b/drivers/gpu/drm/i915/intel_opregion.c >>>> @@ -982,7 +982,9 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv) >>>> opregion->vbt_size = vbt_size; >>>> } else { >>>> vbt = base + OPREGION_VBT_OFFSET; >>>> - vbt_size = OPREGION_ASLE_EXT_OFFSET - OPREGION_VBT_OFFSET; >>>> + vbt_size = (mboxes & MBOX_ASLE_EXT) ? >>>> + OPREGION_ASLE_EXT_OFFSET : OPREGION_SIZE; >>>> + vbt_size -= OPREGION_VBT_OFFSET; >>>> if (intel_bios_is_valid_vbt(vbt, vbt_size)) { >>>> DRM_DEBUG_KMS("Found valid VBT in ACPI OpRegion (Mailbox #4)\n"); >>>> opregion->vbt = vbt; >>> -- Jani Nikula, Intel Open Source Technology Center