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

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