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

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

[Hans de Goede]

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.

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;
-- 
2.9.3

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

[Hans de Goede]

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.

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;
-- 
2.9.3

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

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

[Jani Nikula]

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?
I.e. do you get "Found valid VBT in ACPI OpRegion (Mailbox #4)\n" in
log? If not, which of the debug messages in intel_bios_is_valid_vbt() do
you get?

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".

BR,
Jani.



>
> 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

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

[Jani Nikula]

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?
I.e. do you get "Found valid VBT in ACPI OpRegion (Mailbox #4)\n" in
log? If not, which of the debug messages in intel_bios_is_valid_vbt() do
you get?

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".

BR,
Jani.



>
> 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

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

[Hans de Goede]

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 ?

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;
>

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

[Hans de Goede]

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.

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;
>>

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

[Hans de Goede]

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.

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;
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

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.

I'm sorry. Going through the opregion specs is not something I cherish,
so I've been postponing this... I'll try to check later today.

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

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

[Jani Nikula]

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.

I'm sorry. Going through the opregion specs is not something I cherish,
so I've been postponing this... I'll try to check later today.

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

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

[Jani Nikula]

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

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

[Jani Nikula]

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

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

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 2969 bytes --]

Hi,

On 23-01-17 11:36, Jani Nikula wrote:
> 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...

Ok, I've attached a dump of the opregion on the tablet in question.

Regards,

Hans


[-- Attachment #2: jumper-ezpad-mini3-i915_opregion.gz --]
[-- Type: application/gzip, Size: 2253 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

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

[Jani Nikula]

On Mon, 23 Jan 2017, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 23-01-17 11:36, Jani Nikula wrote:
>> 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...
>
> Ok, I've attached a dump of the opregion on the tablet in question.

Thanks. rvda is not used, VBT is in in the mailbox, and reports size
0x1843, i.e. 67 bytes over the mailbox size of 6 kB. *facepalm*.

I'll try to ask around to see if someone knows what they were thinking.

BR,
Jani.


>
> Regards,
>
> Hans
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Hans de Goede]

Hi Jani,

So any further update on this ? I've just got a report from
an user that my fix also fixes the LCD panel on his (different
model, ACER SW5_017) machine not lighting up:

https://bugzilla.kernel.org/show_bug.cgi?id=155241#c56

Although my fix my be in contradiction with the VBT spec,
I do not see how it can really hurt, it just allows the
VBT to extend into an unused part of the opregion.

Something which windows 10 apparently allows, otherwise
we would not have multiple models shipping with this
problem, so it seems best to just allow this in Linux
too.

Regards,

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

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

[Jani Nikula]

On Sat, 04 Feb 2017, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi Jani,
>
> So any further update on this ? I've just got a report from
> an user that my fix also fixes the LCD panel on his (different
> model, ACER SW5_017) machine not lighting up:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=155241#c56
>
> Although my fix my be in contradiction with the VBT spec,
> I do not see how it can really hurt, it just allows the
> VBT to extend into an unused part of the opregion.
>
> Something which windows 10 apparently allows, otherwise
> we would not have multiple models shipping with this
> problem, so it seems best to just allow this in Linux
> too.

Suffice it to say I'm pretty frustrated with this.

Let's get your patch merged, I'll reply to it with a couple of specific
comments.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

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.
>
> 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;

First, I want a big fat warning comment about what's going on
here. Otherwise someone's bound to "fix" this later on.

Second, per the spec, the ASLE ext mailbox is 1k in size, and there's a
1k reserved region at the end. We probably shouldn't allow VBT to extend
over there. But hey, per the spec we also shouldn't allow VBT to extend
over mailbox #5 either. So if you can't be bothered with that, neither
will I.


BR,
Jani.



>  			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

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

[Jani Nikula]

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.
>
> 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;

First, I want a big fat warning comment about what's going on
here. Otherwise someone's bound to "fix" this later on.

Second, per the spec, the ASLE ext mailbox is 1k in size, and there's a
1k reserved region at the end. We probably shouldn't allow VBT to extend
over there. But hey, per the spec we also shouldn't allow VBT to extend
over mailbox #5 either. So if you can't be bothered with that, neither
will I.


BR,
Jani.



>  			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

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

[Hans de Goede]

Hi,

On 06-02-17 08:04, 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.
>>
>> 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;
>
> First, I want a big fat warning comment about what's going on
> here. Otherwise someone's bound to "fix" this later on.

Done for v2, which I will send shortly.

> Second, per the spec, the ASLE ext mailbox is 1k in size, and there's a
> 1k reserved region at the end. We probably shouldn't allow VBT to extend
> over there. But hey, per the spec we also shouldn't allow VBT to extend
> over mailbox #5 either. So if you can't be bothered with that, neither
> will I.

Hmm, that makes no sense, OPREGION_SIZE is 8192 bytes or 0x2000,
OPREGION_ASLE_EXT_OFFSET is 0x1C00 or 7168 bytes, if there is 1k
reserved after the ASLE ext mailbox then there is exactly 0 bytes
available for the ASLE ext mailbox or OPREGION_SIZE should be 9216
not 8192 (which I don't think so). Maybe the last 1k is either
the ASLE ext mailbox OR reserved ?

Anyways leaving this bit as is for now.

Regards,

Hans

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

[Hans de Goede]

Hi,

On 06-02-17 08:04, 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.
>>
>> 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;
>
> First, I want a big fat warning comment about what's going on
> here. Otherwise someone's bound to "fix" this later on.

Done for v2, which I will send shortly.

> Second, per the spec, the ASLE ext mailbox is 1k in size, and there's a
> 1k reserved region at the end. We probably shouldn't allow VBT to extend
> over there. But hey, per the spec we also shouldn't allow VBT to extend
> over mailbox #5 either. So if you can't be bothered with that, neither
> will I.

Hmm, that makes no sense, OPREGION_SIZE is 8192 bytes or 0x2000,
OPREGION_ASLE_EXT_OFFSET is 0x1C00 or 7168 bytes, if there is 1k
reserved after the ASLE ext mailbox then there is exactly 0 bytes
available for the ASLE ext mailbox or OPREGION_SIZE should be 9216
not 8192 (which I don't think so). Maybe the last 1k is either
the ASLE ext mailbox OR reserved ?

Anyways leaving this bit as is for now.

Regards,

Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Jani Nikula]

On Fri, 10 Feb 2017, Hans de Goede <hdegoede@redhat.com> wrote:
> On 06-02-17 08:04, Jani Nikula wrote:
>> First, I want a big fat warning comment about what's going on
>> here. Otherwise someone's bound to "fix" this later on.
>
> Done for v2, which I will send shortly.

Thanks.

>> Second, per the spec, the ASLE ext mailbox is 1k in size, and there's a
>> 1k reserved region at the end. We probably shouldn't allow VBT to extend
>> over there. But hey, per the spec we also shouldn't allow VBT to extend
>> over mailbox #5 either. So if you can't be bothered with that, neither
>> will I.
>
> Hmm, that makes no sense, OPREGION_SIZE is 8192 bytes or 0x2000,
> OPREGION_ASLE_EXT_OFFSET is 0x1C00 or 7168 bytes, if there is 1k
> reserved after the ASLE ext mailbox then there is exactly 0 bytes
> available for the ASLE ext mailbox or OPREGION_SIZE should be 9216
> not 8192 (which I don't think so). Maybe the last 1k is either
> the ASLE ext mailbox OR reserved ?

You're quite right, that makes no sense. :) Looks like we account for
the reserved part at the end of struct opregion_asle_ext, but the spec
isn't quite clear whether the reserved part is actually part of the
mailbox or not. I'm not quite sure how I managed to confuse myself to
turn that into 1k. Sorry.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center

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

[Jani Nikula]

On Fri, 10 Feb 2017, Hans de Goede <hdegoede@redhat.com> wrote:
> On 06-02-17 08:04, Jani Nikula wrote:
>> First, I want a big fat warning comment about what's going on
>> here. Otherwise someone's bound to "fix" this later on.
>
> Done for v2, which I will send shortly.

Thanks.

>> Second, per the spec, the ASLE ext mailbox is 1k in size, and there's a
>> 1k reserved region at the end. We probably shouldn't allow VBT to extend
>> over there. But hey, per the spec we also shouldn't allow VBT to extend
>> over mailbox #5 either. So if you can't be bothered with that, neither
>> will I.
>
> Hmm, that makes no sense, OPREGION_SIZE is 8192 bytes or 0x2000,
> OPREGION_ASLE_EXT_OFFSET is 0x1C00 or 7168 bytes, if there is 1k
> reserved after the ASLE ext mailbox then there is exactly 0 bytes
> available for the ASLE ext mailbox or OPREGION_SIZE should be 9216
> not 8192 (which I don't think so). Maybe the last 1k is either
> the ASLE ext mailbox OR reserved ?

You're quite right, that makes no sense. :) Looks like we account for
the reserved part at the end of struct opregion_asle_ext, but the spec
isn't quite clear whether the reserved part is actually part of the
mailbox or not. I'm not quite sure how I managed to confuse myself to
turn that into 1k. Sorry.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx