Re: [PATCH] drm/i915: Fix not finding the VBT when it overlaps with OPREGION_ASLE_EXT

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 20 Jan 2017, Hans de Goede <hdegoede@redhat.com> 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 <hdegoede@redhat.com> 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 <hdegoede@redhat.com>
>>>> ---
>>>> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx