Re: [Intel-gfx] [PATCH 1/9] x86/early-quirks: Extend Intel graphics stolen memory placement to 64bit

[Thomas Gleixner]

Matthew,

On Thu, 7 Dec 2017, Matthew Auld wrote:

Can you please add a version number to your patches? Having the same
subject line five times is just annoying.

> From: Joonas Lahtinen 
 
> To give upcoming SKU BIOSes more flexibility in placing the Intel
> graphics stolen memory, make all variables storing the placement or size
> compatible with full 64 bit range. Also by exporting the stolen region
> as a resource, we can then nuke the duplicated stolen discovery in i915.
> 
> v2: export the stolen region as a resource
> fix u16 << 16 (Chris)
> v3: actually fix u16 << 16

And please move the version thing below the --- separator so it can be
discarded by tools. It's not part of the changelog.

> +struct resource intel_graphics_stolen_res = DEFINE_RES_MEM(0, 0);

This is updated in __init so the variable should be marked __ro_after_init.

Thanks,

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

[Intel-gfx] [PATCH 1/9] x86/early-quirks: Extend Intel graphics stolen memory placement to 64bit

[Matthew Auld]

From: Joonas Lahtinen 

To give upcoming SKU BIOSes more flexibility in placing the Intel
graphics stolen memory, make all variables storing the placement or size
compatible with full 64 bit range. Also by exporting the stolen region
as a resource, we can then nuke the duplicated stolen discovery in i915.

v2: export the stolen region as a resource
fix u16 << 16 (Chris)
v3: actually fix u16 << 16

Signed-off-by: Joonas Lahtinen 
Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Ville Syrjälä 
Cc: Chris Wilson 
Cc: Paulo Zanoni 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: x...@kernel.org
Cc: linux-ker...@vger.kernel.org
Reviewed-by: Chris Wilson 
---
 arch/x86/kernel/early-quirks.c | 86 +++---
 include/drm/i915_drm.h |  3 ++
 2 files changed, 50 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 1e82f787c160..b5855b00a8cc 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -243,7 +243,7 @@ static void __init intel_remapping_check(int num, int slot, 
int func)
 #define KB(x)  ((x) * 1024UL)
 #define MB(x)  (KB (KB (x)))
 
-static size_t __init i830_tseg_size(void)
+static resource_size_t __init i830_tseg_size(void)
 {
u8 esmramc = read_pci_config_byte(0, 0, 0, I830_ESMRAMC);
 
@@ -256,7 +256,7 @@ static size_t __init i830_tseg_size(void)
return KB(512);
 }
 
-static size_t __init i845_tseg_size(void)
+static resource_size_t __init i845_tseg_size(void)
 {
u8 esmramc = read_pci_config_byte(0, 0, 0, I845_ESMRAMC);
u8 tseg_size = esmramc & I845_TSEG_SIZE_MASK;
@@ -273,7 +273,7 @@ static size_t __init i845_tseg_size(void)
return 0;
 }
 
-static size_t __init i85x_tseg_size(void)
+static resource_size_t __init i85x_tseg_size(void)
 {
u8 esmramc = read_pci_config_byte(0, 0, 0, I85X_ESMRAMC);
 
@@ -283,12 +283,12 @@ static size_t __init i85x_tseg_size(void)
return MB(1);
 }
 
-static size_t __init i830_mem_size(void)
+static resource_size_t __init i830_mem_size(void)
 {
return read_pci_config_byte(0, 0, 0, I830_DRB3) * MB(32);
 }
 
-static size_t __init i85x_mem_size(void)
+static resource_size_t __init i85x_mem_size(void)
 {
return read_pci_config_byte(0, 0, 1, I85X_DRB3) * MB(32);
 }
@@ -297,36 +297,36 @@ static size_t __init i85x_mem_size(void)
  * On 830/845/85x the stolen memory base isn't available in any
  * register. We need to calculate it as TOM-TSEG_SIZE-stolen_size.
  */
-static phys_addr_t __init i830_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i830_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
-   return (phys_addr_t)i830_mem_size() - i830_tseg_size() - stolen_size;
+   return i830_mem_size() - i830_tseg_size() - stolen_size;
 }
 
-static phys_addr_t __init i845_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i845_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
-   return (phys_addr_t)i830_mem_size() - i845_tseg_size() - stolen_size;
+   return i830_mem_size() - i845_tseg_size() - stolen_size;
 }
 
-static phys_addr_t __init i85x_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i85x_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
-   return (phys_addr_t)i85x_mem_size() - i85x_tseg_size() - stolen_size;
+   return i85x_mem_size() - i85x_tseg_size() - stolen_size;
 }
 
-static phys_addr_t __init i865_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i865_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
u16 toud = 0;
 
toud = read_pci_config_16(0, 0, 0, I865_TOUD);
 
-   return (phys_addr_t)(toud << 16) + i845_tseg_size();
+   return toud * KB(64) + i845_tseg_size();
 }
 
-static phys_addr_t __init gen3_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init gen3_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
u32 bsm;
 
@@ -337,10 +337,10 @@ static 

[Intel-gfx] [PATCH 1/9] x86/early-quirks: Extend Intel graphics stolen memory placement to 64bit

[Matthew Auld]

From: Joonas Lahtinen 

To give upcoming SKU BIOSes more flexibility in placing the Intel
graphics stolen memory, make all variables storing the placement or size
compatible with full 64 bit range. Also by exporting the stolen region
as a resource, we can then nuke the duplicated stolen discovery in i915.

v2: export the stolen region as a resource
fix u16 << 16 (Chris)
v3: actually fix u16 << 16

Signed-off-by: Joonas Lahtinen 
Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Ville Syrjälä 
Cc: Chris Wilson 
Cc: Paulo Zanoni 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: x...@kernel.org
Cc: linux-ker...@vger.kernel.org
Reviewed-by: Chris Wilson  #v1
---
 arch/x86/kernel/early-quirks.c | 86 +++---
 include/drm/i915_drm.h |  3 ++
 2 files changed, 50 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 1e82f787c160..d7236e2f5eed 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -243,7 +243,7 @@ static void __init intel_remapping_check(int num, int slot, 
int func)
 #define KB(x)  ((x) * 1024UL)
 #define MB(x)  (KB (KB (x)))
 
-static size_t __init i830_tseg_size(void)
+static resource_size_t __init i830_tseg_size(void)
 {
u8 esmramc = read_pci_config_byte(0, 0, 0, I830_ESMRAMC);
 
@@ -256,7 +256,7 @@ static size_t __init i830_tseg_size(void)
return KB(512);
 }
 
-static size_t __init i845_tseg_size(void)
+static resource_size_t __init i845_tseg_size(void)
 {
u8 esmramc = read_pci_config_byte(0, 0, 0, I845_ESMRAMC);
u8 tseg_size = esmramc & I845_TSEG_SIZE_MASK;
@@ -273,7 +273,7 @@ static size_t __init i845_tseg_size(void)
return 0;
 }
 
-static size_t __init i85x_tseg_size(void)
+static resource_size_t __init i85x_tseg_size(void)
 {
u8 esmramc = read_pci_config_byte(0, 0, 0, I85X_ESMRAMC);
 
@@ -283,12 +283,12 @@ static size_t __init i85x_tseg_size(void)
return MB(1);
 }
 
-static size_t __init i830_mem_size(void)
+static resource_size_t __init i830_mem_size(void)
 {
return read_pci_config_byte(0, 0, 0, I830_DRB3) * MB(32);
 }
 
-static size_t __init i85x_mem_size(void)
+static resource_size_t __init i85x_mem_size(void)
 {
return read_pci_config_byte(0, 0, 1, I85X_DRB3) * MB(32);
 }
@@ -297,36 +297,36 @@ static size_t __init i85x_mem_size(void)
  * On 830/845/85x the stolen memory base isn't available in any
  * register. We need to calculate it as TOM-TSEG_SIZE-stolen_size.
  */
-static phys_addr_t __init i830_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i830_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
-   return (phys_addr_t)i830_mem_size() - i830_tseg_size() - stolen_size;
+   return i830_mem_size() - i830_tseg_size() - stolen_size;
 }
 
-static phys_addr_t __init i845_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i845_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
-   return (phys_addr_t)i830_mem_size() - i845_tseg_size() - stolen_size;
+   return i830_mem_size() - i845_tseg_size() - stolen_size;
 }
 
-static phys_addr_t __init i85x_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i85x_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
-   return (phys_addr_t)i85x_mem_size() - i85x_tseg_size() - stolen_size;
+   return i85x_mem_size() - i85x_tseg_size() - stolen_size;
 }
 
-static phys_addr_t __init i865_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i865_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
u16 toud = 0;
 
toud = read_pci_config_16(0, 0, 0, I865_TOUD);
 
-   return (phys_addr_t)(toud << 16) + i845_tseg_size();
+   return (toud * KB(64)) + i845_tseg_size();
 }
 
-static phys_addr_t __init gen3_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init gen3_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
u32 bsm;
 
@@ -337,10 +337,10 @@ 

Re: [Intel-gfx] [PATCH 1/9] x86/early-quirks: Extend Intel graphics stolen memory placement to 64bit

[Joonas Lahtinen]

On Fri, 2017-11-24 at 22:05 +, Chris Wilson wrote:
> Quoting Matthew Auld (2017-11-24 21:29:22)
> > From: Joonas Lahtinen 
> > 
> > In preparation for upcoming SKUs, allow more freedom in placement
> > of the Intel graphics stolen memory by BIOS to full 64bit range.
> > 
> > v2: export the stolen region as a resource
> > fix u16 << 16 (Chris)
> > 
> > Signed-off-by: Joonas Lahtinen 
> > Signed-off-by: Matthew Auld 
> > Cc: Joonas Lahtinen 
> > Cc: Ville Syrjälä 
> > Cc: Chris Wilson 
> > Cc: Paulo Zanoni 
> > Cc: Ingo Molnar 
> > Cc: H. Peter Anvin 
> > Cc: x...@kernel.org
> > Reviewed-by: Chris Wilson  #v1
> > ---
> > -static phys_addr_t __init i865_stolen_base(int num, int slot, int func,
> > -  size_t stolen_size)
> > +static resource_size_t __init i865_stolen_base(int num, int slot, int func,
> > +  resource_size_t stolen_size)
> >  {
> > u16 toud = 0;
> >  
> > toud = read_pci_config_16(0, 0, 0, I865_TOUD);
> >  
> > -   return (phys_addr_t)(toud << 16) + i845_tseg_size();
> > +   return (resource_size_t)(toud << 16) + i845_tseg_size();
> 
> We need the cast on toud before the <<.

I was thinking should could convert this into multiplication, while at
it. Does that make sense to you?

return (toud * KB(64)) + i845_tseg_size();

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/9] x86/early-quirks: Extend Intel graphics stolen memory placement to 64bit

[Joonas Lahtinen]

On Sat, 2017-11-25 at 00:48 +0100, Thomas Gleixner wrote:
> On Fri, 24 Nov 2017, Matthew Auld wrote:
> 
> > From: Joonas Lahtinen 
> 
> Please CC the linux kernel mailinglist on patches related to x86. The
> MAINTAINERS file says:
> 
> X86 ARCHITECTURE (32-BIT AND 64-BIT)
> M:  Thomas Gleixner 
> M:  Ingo Molnar 
> M:  "H. Peter Anvin" 
> M:  x...@kernel.org
> L:  linux-ker...@vger.kernel.org  <===
> 
> See also Documentation/process/submitting-patches.rst, chapter:
> 
> 5) Select the recipients for your patch

My bad. I lazily copied over the Cc: list from the patch that was meant
to squashed into this patch.

> > In preparation for upcoming SKUs, allow more freedom in placement
> > of the Intel graphics stolen memory by BIOS to full 64bit range.
> 
> This is not really informative for people not familiar with the issue you
> want to solve.

"To give upcoming SKU BIOSes more flexibility in placing the Intel
graphics stolen memory, make all variables storing the placement or
size compatible with full 64 bit range."

Does that sound better to you?

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/9] x86/early-quirks: Extend Intel graphics stolen memory placement to 64bit

[Thomas Gleixner]

On Fri, 24 Nov 2017, Matthew Auld wrote:

> From: Joonas Lahtinen 

Please CC the linux kernel mailinglist on patches related to x86. The
MAINTAINERS file says:

X86 ARCHITECTURE (32-BIT AND 64-BIT)
M:  Thomas Gleixner 
M:  Ingo Molnar 
M:  "H. Peter Anvin" 
M:  x...@kernel.org
L:  linux-ker...@vger.kernel.org<===

See also Documentation/process/submitting-patches.rst, chapter:

5) Select the recipients for your patch

> In preparation for upcoming SKUs, allow more freedom in placement
> of the Intel graphics stolen memory by BIOS to full 64bit range.

This is not really informative for people not familiar with the issue you
want to solve.

Thanks,

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

Re: [Intel-gfx] [PATCH 1/9] x86/early-quirks: Extend Intel graphics stolen memory placement to 64bit

[Chris Wilson]

Quoting Matthew Auld (2017-11-24 21:29:22)
> From: Joonas Lahtinen 
> 
> In preparation for upcoming SKUs, allow more freedom in placement
> of the Intel graphics stolen memory by BIOS to full 64bit range.
> 
> v2: export the stolen region as a resource
> fix u16 << 16 (Chris)
> 
> Signed-off-by: Joonas Lahtinen 
> Signed-off-by: Matthew Auld 
> Cc: Joonas Lahtinen 
> Cc: Ville Syrjälä 
> Cc: Chris Wilson 
> Cc: Paulo Zanoni 
> Cc: Ingo Molnar 
> Cc: H. Peter Anvin 
> Cc: x...@kernel.org
> Reviewed-by: Chris Wilson  #v1
> ---
> -static phys_addr_t __init i865_stolen_base(int num, int slot, int func,
> -  size_t stolen_size)
> +static resource_size_t __init i865_stolen_base(int num, int slot, int func,
> +  resource_size_t stolen_size)
>  {
> u16 toud = 0;
>  
> toud = read_pci_config_16(0, 0, 0, I865_TOUD);
>  
> -   return (phys_addr_t)(toud << 16) + i845_tseg_size();
> +   return (resource_size_t)(toud << 16) + i845_tseg_size();

We need the cast on toud before the <<.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/9] x86/early-quirks: Extend Intel graphics stolen memory placement to 64bit

[Matthew Auld]

From: Joonas Lahtinen 

In preparation for upcoming SKUs, allow more freedom in placement
of the Intel graphics stolen memory by BIOS to full 64bit range.

v2: export the stolen region as a resource
fix u16 << 16 (Chris)

Signed-off-by: Joonas Lahtinen 
Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Ville Syrjälä 
Cc: Chris Wilson 
Cc: Paulo Zanoni 
Cc: Ingo Molnar 
Cc: H. Peter Anvin 
Cc: x...@kernel.org
Reviewed-by: Chris Wilson  #v1
---
 arch/x86/kernel/early-quirks.c | 86 +++---
 include/drm/i915_drm.h |  3 ++
 2 files changed, 50 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 1e82f787c160..3116f579841a 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -243,7 +243,7 @@ static void __init intel_remapping_check(int num, int slot, 
int func)
 #define KB(x)  ((x) * 1024UL)
 #define MB(x)  (KB (KB (x)))
 
-static size_t __init i830_tseg_size(void)
+static resource_size_t __init i830_tseg_size(void)
 {
u8 esmramc = read_pci_config_byte(0, 0, 0, I830_ESMRAMC);
 
@@ -256,7 +256,7 @@ static size_t __init i830_tseg_size(void)
return KB(512);
 }
 
-static size_t __init i845_tseg_size(void)
+static resource_size_t __init i845_tseg_size(void)
 {
u8 esmramc = read_pci_config_byte(0, 0, 0, I845_ESMRAMC);
u8 tseg_size = esmramc & I845_TSEG_SIZE_MASK;
@@ -273,7 +273,7 @@ static size_t __init i845_tseg_size(void)
return 0;
 }
 
-static size_t __init i85x_tseg_size(void)
+static resource_size_t __init i85x_tseg_size(void)
 {
u8 esmramc = read_pci_config_byte(0, 0, 0, I85X_ESMRAMC);
 
@@ -283,12 +283,12 @@ static size_t __init i85x_tseg_size(void)
return MB(1);
 }
 
-static size_t __init i830_mem_size(void)
+static resource_size_t __init i830_mem_size(void)
 {
return read_pci_config_byte(0, 0, 0, I830_DRB3) * MB(32);
 }
 
-static size_t __init i85x_mem_size(void)
+static resource_size_t __init i85x_mem_size(void)
 {
return read_pci_config_byte(0, 0, 1, I85X_DRB3) * MB(32);
 }
@@ -297,36 +297,36 @@ static size_t __init i85x_mem_size(void)
  * On 830/845/85x the stolen memory base isn't available in any
  * register. We need to calculate it as TOM-TSEG_SIZE-stolen_size.
  */
-static phys_addr_t __init i830_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i830_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
-   return (phys_addr_t)i830_mem_size() - i830_tseg_size() - stolen_size;
+   return i830_mem_size() - i830_tseg_size() - stolen_size;
 }
 
-static phys_addr_t __init i845_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i845_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
-   return (phys_addr_t)i830_mem_size() - i845_tseg_size() - stolen_size;
+   return i830_mem_size() - i845_tseg_size() - stolen_size;
 }
 
-static phys_addr_t __init i85x_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i85x_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
-   return (phys_addr_t)i85x_mem_size() - i85x_tseg_size() - stolen_size;
+   return i85x_mem_size() - i85x_tseg_size() - stolen_size;
 }
 
-static phys_addr_t __init i865_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init i865_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
u16 toud = 0;
 
toud = read_pci_config_16(0, 0, 0, I865_TOUD);
 
-   return (phys_addr_t)(toud << 16) + i845_tseg_size();
+   return (resource_size_t)(toud << 16) + i845_tseg_size();
 }
 
-static phys_addr_t __init gen3_stolen_base(int num, int slot, int func,
-  size_t stolen_size)
+static resource_size_t __init gen3_stolen_base(int num, int slot, int func,
+  resource_size_t stolen_size)
 {
u32 bsm;
 
@@ -337,10 +337,10 @@ static phys_addr_t __init gen3_stolen_base(int num, int 
slot, int func,
 */
bsm = read_pci_config(num, slot, func, INTEL_BSM);
 
-   return (phys_addr_t)bsm & INTEL_BSM_MASK;
+   return bsm & INTEL_BSM_MASK;
 }