Re: [Intel-gfx] [PATCH 11/11] drm/i915/opregion: handle VBT sizes bigger than 6 KB

2015-12-17 Thread Jani Nikula
On Thu, 17 Dec 2015, Mika Kahola  wrote:
> On Mon, 2015-12-14 at 12:50 +0200, Jani Nikula wrote:
>> The RVDA and RVDS (raw VBT data address and size) fields of the ASLE
>> mailbox may specify an alternate location for VBT instead of mailbox #4.
>> Use the alternate location if available and valid, falling back to
>> mailbox #4 otherwise.
>> 
> Tested-by: Mika Kahola 
>> Signed-off-by: Jani Nikula 

Pushed to drm-intel-next-queued, thanks for the review and testing.

BR,
Jani.


>> ---
>>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>>  drivers/gpu/drm/i915/intel_opregion.c | 25 +++--
>>  2 files changed, 24 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index ca8c2a64bc6d..8cfac7398568 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -457,6 +457,7 @@ struct intel_opregion {
>>  u32 swsci_gbda_sub_functions;
>>  u32 swsci_sbcb_sub_functions;
>>  struct opregion_asle *asle;
>> +void *rvda;
>>  const void *vbt;
>>  u32 vbt_size;
>>  u32 *lid_state;
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
>> b/drivers/gpu/drm/i915/intel_opregion.c
>> index e89ee2383fe1..a139889dd45b 100644
>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> @@ -837,6 +837,10 @@ void intel_opregion_fini(struct drm_device *dev)
>>  
>>  /* just clear all opregion memory pointers now */
>>  memunmap(opregion->header);
>> +if (opregion->rvda) {
>> +memunmap(opregion->rvda);
>> +opregion->rvda = NULL;
>> +}
>>  opregion->header = NULL;
>>  opregion->acpi = NULL;
>>  opregion->swsci = NULL;
>> @@ -987,12 +991,29 @@ int intel_opregion_setup(struct drm_device *dev)
>>  DRM_DEBUG_DRIVER("ASLE extension supported\n");
>>  
>>  if (!dmi_check_system(intel_no_opregion_vbt)) {
>> -const void *vbt = base + OPREGION_VBT_OFFSET;
>> -u32 vbt_size = OPREGION_ASLE_EXT_OFFSET - OPREGION_VBT_OFFSET;
>> +const void *vbt = NULL;
>> +u32 vbt_size = 0;
>> +
>> +if (opregion->header->opregion_ver >= 2 && opregion->asle &&
>> +opregion->asle->rvda && opregion->asle->rvds) {
>> +opregion->rvda = memremap(opregion->asle->rvda,
>> +  opregion->asle->rvds,
>> +  MEMREMAP_WB);
>> +vbt = opregion->rvda;
>> +vbt_size = opregion->asle->rvds;
>> +}
>>  
>>  if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
>>  opregion->vbt = vbt;
>>  opregion->vbt_size = vbt_size;
>> +DRM_DEBUG_DRIVER("VBT from RVDA\n");
>> +} else {
>> +vbt = base + OPREGION_VBT_OFFSET;
>> +vbt_size = OPREGION_ASLE_EXT_OFFSET - 
>> OPREGION_VBT_OFFSET;
>> +if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
>> +opregion->vbt = vbt;
>> +opregion->vbt_size = vbt_size;
>> +}
>>  }
>>  }
>>  
>
>

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


Re: [Intel-gfx] [PATCH 11/11] drm/i915/opregion: handle VBT sizes bigger than 6 KB

2015-12-16 Thread Mika Kahola
On Mon, 2015-12-14 at 12:50 +0200, Jani Nikula wrote:
> The RVDA and RVDS (raw VBT data address and size) fields of the ASLE
> mailbox may specify an alternate location for VBT instead of mailbox #4.
> Use the alternate location if available and valid, falling back to
> mailbox #4 otherwise.
> 
Tested-by: Mika Kahola 
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_opregion.c | 25 +++--
>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ca8c2a64bc6d..8cfac7398568 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -457,6 +457,7 @@ struct intel_opregion {
>   u32 swsci_gbda_sub_functions;
>   u32 swsci_sbcb_sub_functions;
>   struct opregion_asle *asle;
> + void *rvda;
>   const void *vbt;
>   u32 vbt_size;
>   u32 *lid_state;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> b/drivers/gpu/drm/i915/intel_opregion.c
> index e89ee2383fe1..a139889dd45b 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -837,6 +837,10 @@ void intel_opregion_fini(struct drm_device *dev)
>  
>   /* just clear all opregion memory pointers now */
>   memunmap(opregion->header);
> + if (opregion->rvda) {
> + memunmap(opregion->rvda);
> + opregion->rvda = NULL;
> + }
>   opregion->header = NULL;
>   opregion->acpi = NULL;
>   opregion->swsci = NULL;
> @@ -987,12 +991,29 @@ int intel_opregion_setup(struct drm_device *dev)
>   DRM_DEBUG_DRIVER("ASLE extension supported\n");
>  
>   if (!dmi_check_system(intel_no_opregion_vbt)) {
> - const void *vbt = base + OPREGION_VBT_OFFSET;
> - u32 vbt_size = OPREGION_ASLE_EXT_OFFSET - OPREGION_VBT_OFFSET;
> + const void *vbt = NULL;
> + u32 vbt_size = 0;
> +
> + if (opregion->header->opregion_ver >= 2 && opregion->asle &&
> + opregion->asle->rvda && opregion->asle->rvds) {
> + opregion->rvda = memremap(opregion->asle->rvda,
> +   opregion->asle->rvds,
> +   MEMREMAP_WB);
> + vbt = opregion->rvda;
> + vbt_size = opregion->asle->rvds;
> + }
>  
>   if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
>   opregion->vbt = vbt;
>   opregion->vbt_size = vbt_size;
> + DRM_DEBUG_DRIVER("VBT from RVDA\n");
> + } else {
> + vbt = base + OPREGION_VBT_OFFSET;
> + vbt_size = OPREGION_ASLE_EXT_OFFSET - 
> OPREGION_VBT_OFFSET;
> + if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
> + opregion->vbt = vbt;
> + opregion->vbt_size = vbt_size;
> + }
>   }
>   }
>  


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


Re: [Intel-gfx] [PATCH 11/11] drm/i915/opregion: handle VBT sizes bigger than 6 KB

2015-12-15 Thread Ville Syrjälä
On Mon, Dec 14, 2015 at 04:34:34PM +0200, Ville Syrjälä wrote:
> On Mon, Dec 14, 2015 at 04:19:50PM +0200, Ville Syrjälä wrote:
> > On Mon, Dec 14, 2015 at 12:50:55PM +0200, Jani Nikula wrote:
> > > The RVDA and RVDS (raw VBT data address and size) fields of the ASLE
> > > mailbox may specify an alternate location for VBT instead of mailbox #4.
> > > Use the alternate location if available and valid, falling back to
> > > mailbox #4 otherwise.
> > > 
> > > Signed-off-by: Jani Nikula 
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h   |  1 +
> > >  drivers/gpu/drm/i915/intel_opregion.c | 25 +++--
> > >  2 files changed, 24 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index ca8c2a64bc6d..8cfac7398568 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -457,6 +457,7 @@ struct intel_opregion {
> > >   u32 swsci_gbda_sub_functions;
> > >   u32 swsci_sbcb_sub_functions;
> > >   struct opregion_asle *asle;
> > > + void *rvda;
> > >   const void *vbt;
> > >   u32 vbt_size;
> > >   u32 *lid_state;
> > > diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> > > b/drivers/gpu/drm/i915/intel_opregion.c
> > > index e89ee2383fe1..a139889dd45b 100644
> > > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > > @@ -837,6 +837,10 @@ void intel_opregion_fini(struct drm_device *dev)
> > >  
> > >   /* just clear all opregion memory pointers now */
> > >   memunmap(opregion->header);
> > > + if (opregion->rvda) {
> > > + memunmap(opregion->rvda);
> > > + opregion->rvda = NULL;
> > > + }
> > >   opregion->header = NULL;
> > >   opregion->acpi = NULL;
> > >   opregion->swsci = NULL;
> > > @@ -987,12 +991,29 @@ int intel_opregion_setup(struct drm_device *dev)
> > >   `   DRM_DEBUG_DRIVER("ASLE extension supported\n");
> > >  
> > >   if (!dmi_check_system(intel_no_opregion_vbt)) {
> > > - const void *vbt = base + OPREGION_VBT_OFFSET;
> > > - u32 vbt_size = OPREGION_ASLE_EXT_OFFSET - OPREGION_VBT_OFFSET;
> > > + const void *vbt = NULL;
> > > + u32 vbt_size = 0;
> > > +
> > > + if (opregion->header->opregion_ver >= 2 && opregion->asle &&
> > > + opregion->asle->rvda && opregion->asle->rvds) {
> > 
> > Either I'm blind or you didn't actually add rvda/rvds to struct 
> > opregion_asle.
> 
> Oh I se they went in seprately as
> c85f6c91ec42 ("drm/i915: add VBT address and size fields to ASLE mailbox 
> struct")
> 
> OK, so apart from the few minor bikesheds I listed it all looks good to
> me. For the series
> Reviewed-by: Ville Syrjälä 

r-b still holds for the patches with the revised debug prints.

> 
> > 
> > Also the spec seems confused as usual. Some parts if it refer to this
> > as RVDA others as RVBT. Although RVBT also seems to be what the mbox #4
> > contents are called in another place, and to add insult to injury that
> > place also has the offset and size all wrong. Sigh.
> > 
> > Anyway, apart from the missing rvda/rvds definititions the rest looks OK.
> > 
> > > + opregion->rvda = memremap(opregion->asle->rvda,
> > > +   opregion->asle->rvds,
> > > +   MEMREMAP_WB);
> > > + vbt = opregion->rvda;
> > > + vbt_size = opregion->asle->rvds;
> > > + }
> > >  
> > >   if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
> > >   opregion->vbt = vbt;
> > >   opregion->vbt_size = vbt_size;
> > > + DRM_DEBUG_DRIVER("VBT from RVDA\n");
> > > + } else {
> > > + vbt = base + OPREGION_VBT_OFFSET;
> > > + vbt_size = OPREGION_ASLE_EXT_OFFSET - 
> > > OPREGION_VBT_OFFSET;
> > > + if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
> > > + opregion->vbt = vbt;
> > > + opregion->vbt_size = vbt_size;
> > > + }
> > >   }
> > >   }
> > >  
> > > -- 
> > > 2.1.4
> > > 
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

[Intel-gfx] [PATCH 11/11] drm/i915/opregion: handle VBT sizes bigger than 6 KB

2015-12-14 Thread Jani Nikula
The RVDA and RVDS (raw VBT data address and size) fields of the ASLE
mailbox may specify an alternate location for VBT instead of mailbox #4.
Use the alternate location if available and valid, falling back to
mailbox #4 otherwise.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_opregion.c | 25 +++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ca8c2a64bc6d..8cfac7398568 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -457,6 +457,7 @@ struct intel_opregion {
u32 swsci_gbda_sub_functions;
u32 swsci_sbcb_sub_functions;
struct opregion_asle *asle;
+   void *rvda;
const void *vbt;
u32 vbt_size;
u32 *lid_state;
diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index e89ee2383fe1..a139889dd45b 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -837,6 +837,10 @@ void intel_opregion_fini(struct drm_device *dev)
 
/* just clear all opregion memory pointers now */
memunmap(opregion->header);
+   if (opregion->rvda) {
+   memunmap(opregion->rvda);
+   opregion->rvda = NULL;
+   }
opregion->header = NULL;
opregion->acpi = NULL;
opregion->swsci = NULL;
@@ -987,12 +991,29 @@ int intel_opregion_setup(struct drm_device *dev)
DRM_DEBUG_DRIVER("ASLE extension supported\n");
 
if (!dmi_check_system(intel_no_opregion_vbt)) {
-   const void *vbt = base + OPREGION_VBT_OFFSET;
-   u32 vbt_size = OPREGION_ASLE_EXT_OFFSET - OPREGION_VBT_OFFSET;
+   const void *vbt = NULL;
+   u32 vbt_size = 0;
+
+   if (opregion->header->opregion_ver >= 2 && opregion->asle &&
+   opregion->asle->rvda && opregion->asle->rvds) {
+   opregion->rvda = memremap(opregion->asle->rvda,
+ opregion->asle->rvds,
+ MEMREMAP_WB);
+   vbt = opregion->rvda;
+   vbt_size = opregion->asle->rvds;
+   }
 
if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
opregion->vbt = vbt;
opregion->vbt_size = vbt_size;
+   DRM_DEBUG_DRIVER("VBT from RVDA\n");
+   } else {
+   vbt = base + OPREGION_VBT_OFFSET;
+   vbt_size = OPREGION_ASLE_EXT_OFFSET - 
OPREGION_VBT_OFFSET;
+   if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
+   opregion->vbt = vbt;
+   opregion->vbt_size = vbt_size;
+   }
}
}
 
-- 
2.1.4

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


Re: [Intel-gfx] [PATCH 11/11] drm/i915/opregion: handle VBT sizes bigger than 6 KB

2015-12-14 Thread Jani Nikula
On Mon, 14 Dec 2015, Ville Syrjälä  wrote:
> On Mon, Dec 14, 2015 at 12:50:55PM +0200, Jani Nikula wrote:
>> The RVDA and RVDS (raw VBT data address and size) fields of the ASLE
>> mailbox may specify an alternate location for VBT instead of mailbox #4.
>> Use the alternate location if available and valid, falling back to
>> mailbox #4 otherwise.
>> 
>> Signed-off-by: Jani Nikula 
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>>  drivers/gpu/drm/i915/intel_opregion.c | 25 +++--
>>  2 files changed, 24 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index ca8c2a64bc6d..8cfac7398568 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -457,6 +457,7 @@ struct intel_opregion {
>>  u32 swsci_gbda_sub_functions;
>>  u32 swsci_sbcb_sub_functions;
>>  struct opregion_asle *asle;
>> +void *rvda;
>>  const void *vbt;
>>  u32 vbt_size;
>>  u32 *lid_state;
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
>> b/drivers/gpu/drm/i915/intel_opregion.c
>> index e89ee2383fe1..a139889dd45b 100644
>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> @@ -837,6 +837,10 @@ void intel_opregion_fini(struct drm_device *dev)
>>  
>>  /* just clear all opregion memory pointers now */
>>  memunmap(opregion->header);
>> +if (opregion->rvda) {
>> +memunmap(opregion->rvda);
>> +opregion->rvda = NULL;
>> +}
>>  opregion->header = NULL;
>>  opregion->acpi = NULL;
>>  opregion->swsci = NULL;
>> @@ -987,12 +991,29 @@ int intel_opregion_setup(struct drm_device *dev)
>>  `   DRM_DEBUG_DRIVER("ASLE extension supported\n");
>>  
>>  if (!dmi_check_system(intel_no_opregion_vbt)) {
>> -const void *vbt = base + OPREGION_VBT_OFFSET;
>> -u32 vbt_size = OPREGION_ASLE_EXT_OFFSET - OPREGION_VBT_OFFSET;
>> +const void *vbt = NULL;
>> +u32 vbt_size = 0;
>> +
>> +if (opregion->header->opregion_ver >= 2 && opregion->asle &&
>> +opregion->asle->rvda && opregion->asle->rvds) {
>
> Either I'm blind or you didn't actually add rvda/rvds to struct opregion_asle.
>
> Also the spec seems confused as usual. Some parts if it refer to this
> as RVDA others as RVBT. Although RVBT also seems to be what the mbox #4
> contents are called in another place, and to add insult to injury that
> place also has the offset and size all wrong. Sigh.
>
> Anyway, apart from the missing rvda/rvds definititions the rest looks OK.

Those are in Deepak's commit I pushed last week:

commit c85f6c91ec4218487a29a7d1f918f7f6f0424c75
Author: Deepak M 
Date:   Tue Dec 1 04:17:05 2015 +0530

drm/i915: add VBT address and size fields to ASLE mailbox struct

I did build this, and even booted it. ;)


BR,
Jani.


>
>> +opregion->rvda = memremap(opregion->asle->rvda,
>> +  opregion->asle->rvds,
>> +  MEMREMAP_WB);
>> +vbt = opregion->rvda;
>> +vbt_size = opregion->asle->rvds;
>> +}
>>  
>>  if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
>>  opregion->vbt = vbt;
>>  opregion->vbt_size = vbt_size;
>> +DRM_DEBUG_DRIVER("VBT from RVDA\n");
>> +} else {
>> +vbt = base + OPREGION_VBT_OFFSET;
>> +vbt_size = OPREGION_ASLE_EXT_OFFSET - 
>> OPREGION_VBT_OFFSET;
>> +if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
>> +opregion->vbt = vbt;
>> +opregion->vbt_size = vbt_size;
>> +}
>>  }
>>  }
>>  
>> -- 
>> 2.1.4
>> 
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH 11/11] drm/i915/opregion: handle VBT sizes bigger than 6 KB

2015-12-14 Thread Ville Syrjälä
On Mon, Dec 14, 2015 at 12:50:55PM +0200, Jani Nikula wrote:
> The RVDA and RVDS (raw VBT data address and size) fields of the ASLE
> mailbox may specify an alternate location for VBT instead of mailbox #4.
> Use the alternate location if available and valid, falling back to
> mailbox #4 otherwise.
> 
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_opregion.c | 25 +++--
>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ca8c2a64bc6d..8cfac7398568 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -457,6 +457,7 @@ struct intel_opregion {
>   u32 swsci_gbda_sub_functions;
>   u32 swsci_sbcb_sub_functions;
>   struct opregion_asle *asle;
> + void *rvda;
>   const void *vbt;
>   u32 vbt_size;
>   u32 *lid_state;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> b/drivers/gpu/drm/i915/intel_opregion.c
> index e89ee2383fe1..a139889dd45b 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -837,6 +837,10 @@ void intel_opregion_fini(struct drm_device *dev)
>  
>   /* just clear all opregion memory pointers now */
>   memunmap(opregion->header);
> + if (opregion->rvda) {
> + memunmap(opregion->rvda);
> + opregion->rvda = NULL;
> + }
>   opregion->header = NULL;
>   opregion->acpi = NULL;
>   opregion->swsci = NULL;
> @@ -987,12 +991,29 @@ int intel_opregion_setup(struct drm_device *dev)
>   `   DRM_DEBUG_DRIVER("ASLE extension supported\n");
>  
>   if (!dmi_check_system(intel_no_opregion_vbt)) {
> - const void *vbt = base + OPREGION_VBT_OFFSET;
> - u32 vbt_size = OPREGION_ASLE_EXT_OFFSET - OPREGION_VBT_OFFSET;
> + const void *vbt = NULL;
> + u32 vbt_size = 0;
> +
> + if (opregion->header->opregion_ver >= 2 && opregion->asle &&
> + opregion->asle->rvda && opregion->asle->rvds) {

Either I'm blind or you didn't actually add rvda/rvds to struct opregion_asle.

Also the spec seems confused as usual. Some parts if it refer to this
as RVDA others as RVBT. Although RVBT also seems to be what the mbox #4
contents are called in another place, and to add insult to injury that
place also has the offset and size all wrong. Sigh.

Anyway, apart from the missing rvda/rvds definititions the rest looks OK.

> + opregion->rvda = memremap(opregion->asle->rvda,
> +   opregion->asle->rvds,
> +   MEMREMAP_WB);
> + vbt = opregion->rvda;
> + vbt_size = opregion->asle->rvds;
> + }
>  
>   if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
>   opregion->vbt = vbt;
>   opregion->vbt_size = vbt_size;
> + DRM_DEBUG_DRIVER("VBT from RVDA\n");
> + } else {
> + vbt = base + OPREGION_VBT_OFFSET;
> + vbt_size = OPREGION_ASLE_EXT_OFFSET - 
> OPREGION_VBT_OFFSET;
> + if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
> + opregion->vbt = vbt;
> + opregion->vbt_size = vbt_size;
> + }
>   }
>   }
>  
> -- 
> 2.1.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 11/11] drm/i915/opregion: handle VBT sizes bigger than 6 KB

2015-12-14 Thread Ville Syrjälä
On Mon, Dec 14, 2015 at 04:19:50PM +0200, Ville Syrjälä wrote:
> On Mon, Dec 14, 2015 at 12:50:55PM +0200, Jani Nikula wrote:
> > The RVDA and RVDS (raw VBT data address and size) fields of the ASLE
> > mailbox may specify an alternate location for VBT instead of mailbox #4.
> > Use the alternate location if available and valid, falling back to
> > mailbox #4 otherwise.
> > 
> > Signed-off-by: Jani Nikula 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h   |  1 +
> >  drivers/gpu/drm/i915/intel_opregion.c | 25 +++--
> >  2 files changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index ca8c2a64bc6d..8cfac7398568 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -457,6 +457,7 @@ struct intel_opregion {
> > u32 swsci_gbda_sub_functions;
> > u32 swsci_sbcb_sub_functions;
> > struct opregion_asle *asle;
> > +   void *rvda;
> > const void *vbt;
> > u32 vbt_size;
> > u32 *lid_state;
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> > b/drivers/gpu/drm/i915/intel_opregion.c
> > index e89ee2383fe1..a139889dd45b 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -837,6 +837,10 @@ void intel_opregion_fini(struct drm_device *dev)
> >  
> > /* just clear all opregion memory pointers now */
> > memunmap(opregion->header);
> > +   if (opregion->rvda) {
> > +   memunmap(opregion->rvda);
> > +   opregion->rvda = NULL;
> > +   }
> > opregion->header = NULL;
> > opregion->acpi = NULL;
> > opregion->swsci = NULL;
> > @@ -987,12 +991,29 @@ int intel_opregion_setup(struct drm_device *dev)
> > `   DRM_DEBUG_DRIVER("ASLE extension supported\n");
> >  
> > if (!dmi_check_system(intel_no_opregion_vbt)) {
> > -   const void *vbt = base + OPREGION_VBT_OFFSET;
> > -   u32 vbt_size = OPREGION_ASLE_EXT_OFFSET - OPREGION_VBT_OFFSET;
> > +   const void *vbt = NULL;
> > +   u32 vbt_size = 0;
> > +
> > +   if (opregion->header->opregion_ver >= 2 && opregion->asle &&
> > +   opregion->asle->rvda && opregion->asle->rvds) {
> 
> Either I'm blind or you didn't actually add rvda/rvds to struct opregion_asle.

Oh I se they went in seprately as
c85f6c91ec42 ("drm/i915: add VBT address and size fields to ASLE mailbox 
struct")

OK, so apart from the few minor bikesheds I listed it all looks good to
me. For the series
Reviewed-by: Ville Syrjälä 

> 
> Also the spec seems confused as usual. Some parts if it refer to this
> as RVDA others as RVBT. Although RVBT also seems to be what the mbox #4
> contents are called in another place, and to add insult to injury that
> place also has the offset and size all wrong. Sigh.
> 
> Anyway, apart from the missing rvda/rvds definititions the rest looks OK.
> 
> > +   opregion->rvda = memremap(opregion->asle->rvda,
> > + opregion->asle->rvds,
> > + MEMREMAP_WB);
> > +   vbt = opregion->rvda;
> > +   vbt_size = opregion->asle->rvds;
> > +   }
> >  
> > if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
> > opregion->vbt = vbt;
> > opregion->vbt_size = vbt_size;
> > +   DRM_DEBUG_DRIVER("VBT from RVDA\n");
> > +   } else {
> > +   vbt = base + OPREGION_VBT_OFFSET;
> > +   vbt_size = OPREGION_ASLE_EXT_OFFSET - 
> > OPREGION_VBT_OFFSET;
> > +   if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
> > +   opregion->vbt = vbt;
> > +   opregion->vbt_size = vbt_size;
> > +   }
> > }
> > }
> >  
> > -- 
> > 2.1.4
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx