Re: [Intel-gfx] [PATCH] drm/i915: Read the Base of Stolen Memory for 915gm

2013-02-19 Thread Chris Wilson
On Wed, Feb 20, 2013 at 12:28:00AM +0100, Daniel Vetter wrote:
> Hm, sloppy me didn't send out the "patch merged" spam. Anyway:
> 
> stolen + phys_object = kaboom

Nice reminder that the phys objects need to be upgraded to stolen.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Read the Base of Stolen Memory for 915gm

2013-02-19 Thread Daniel Vetter
On Sun, Feb 10, 2013 at 01:37:52PM -0800, Ben Widawsky wrote:
> On Sun, Feb 10, 2013 at 07:38:13PM +, Chris Wilson wrote:
> > Reading the cspec pays dividends once again, as I found the 'Base of
> > Stolen Memory' config register so that we can skip the fragile
> > computation from Top of Usable Draw and use the real value provided by
> > the BIOS.
> > 
> > Signed-off-by: Chris Wilson 
> Don't really want to dig out the doc, but I so prefer this to the
> old way.
> Acked-by: Ben Widawsky 

Hm, sloppy me didn't send out the "patch merged" spam. Anyway:

stolen + phys_object = kaboom

Dropped the patcha again.
-Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_stolen.c |9 +++--
> >  1 file changed, 3 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> > b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index 130d1db..7f1735c 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -72,13 +72,10 @@ static unsigned long i915_stolen_to_physical(struct 
> > drm_device *dev)
> > } else if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) {
> > /* Read Graphics Base of Stolen Memory directly */
> > pci_read_config_dword(pdev, 0xA4, &base);
> > -#if 0
> > } else if (IS_GEN3(dev)) {
> > -   u8 val;
> > -   /* Stolen is immediately below Top of Low Usable DRAM */
> > -   pci_read_config_byte(pdev, 0x9c, &val);
> > -   base = val >> 3 << 27;
> > -   base -= dev_priv->mm.gtt->stolen_size;
> > +   /* Read D2:F0 Base of Stolen Memory directly */
> > +   pci_read_config_dword(dev->pdev, 0x5c, &base);
> > +#if 0
> > } else {
> > /* Stolen is immediately above Top of Memory */
> > base = max_low_pfn_mapped << PAGE_SHIFT;
> > -- 
> > 1.7.10.4
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Read the Base of Stolen Memory for 915gm

2013-02-11 Thread Paul Menzel
Dear Chris,


Am Sonntag, den 10.02.2013, 23:21 + schrieb Chris Wilson:
> On Sun, Feb 10, 2013 at 11:37:03PM +0100, Paul Menzel wrote:
> > Am Sonntag, den 10.02.2013, 19:38 + schrieb Chris Wilson:
> > > Reading the cspec pays dividends once again, as I found the 'Base of
> > > Stolen Memory' config register so that we can skip the fragile
> > > computation from Top of Usable Draw and use the real value provided by
> > > the BIOS.
> > 
> > Nice find. What is the expected functionality change as the code
> > beforehand was a no-op due to the `if 0`?
> 
> It enables the driver to use stolen memory - memory that is hidden from
> the system and only available, in principal, to the gfx device.

ah, nice. Thank you for the explanation.

Should it be backported to the stable Linux kernel releases then, by
adding the following line to the commit message?

CC: sta...@vger.kernel.org

> > If testing is needed, how can this be tested?
> 
> Boot a 915g/945g. The driver will utilize stolen memory for its
> ringbuffers and fbcon.

Any messages which will get logged to see if it works correctly?

> > > - /* Stolen is immediately below Top of Low Usable DRAM */
> > > - pci_read_config_byte(pdev, 0x9c, &val);
> > > - base = val >> 3 << 27;
> > > - base -= dev_priv->mm.gtt->stolen_size;
> > > + /* Read D2:F0 Base of Stolen Memory directly */
> > > + pci_read_config_dword(dev->pdev, 0x5c, &base);
> > 
> > Should macros be defined for the register names?
> 
> My argument is basically where else are you planning to use this value.
> Each chipset calls it something different and uses a different register,
> so DRY is not violated and instead of hiding the value in a header, we
> can define it locally and clearly. Makes adapting it for the next
> generation much easier as you only have the single location to worry
> about. (The normal argument for the macro, but in reverse.)

Sounds reasonable.


Thanks,

Paul


signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Read the Base of Stolen Memory for 915gm

2013-02-10 Thread Chris Wilson
On Sun, Feb 10, 2013 at 11:37:03PM +0100, Paul Menzel wrote:
> Am Sonntag, den 10.02.2013, 19:38 + schrieb Chris Wilson:
> > Reading the cspec pays dividends once again, as I found the 'Base of
> > Stolen Memory' config register so that we can skip the fragile
> > computation from Top of Usable Draw and use the real value provided by
> > the BIOS.
> 
> Nice find. What is the expected functionality change as the code
> beforehand was a no-op due to the `if 0`?

It enables the driver to use stolen memory - memory that is hidden from
the system and only available, in principal, to the gfx device.
 
> If testing is needed, how can this be tested?

Boot a 915g/945g. The driver will utilize stolen memory for its
ringbuffers and fbcon.

> > -   /* Stolen is immediately below Top of Low Usable DRAM */
> > -   pci_read_config_byte(pdev, 0x9c, &val);
> > -   base = val >> 3 << 27;
> > -   base -= dev_priv->mm.gtt->stolen_size;
> > +   /* Read D2:F0 Base of Stolen Memory directly */
> > +   pci_read_config_dword(dev->pdev, 0x5c, &base);
> 
> Should macros be defined for the register names?

My argument is basically where else are you planning to use this value.
Each chipset calls it something different and uses a different register,
so DRY is not violated and instead of hiding the value in a header, we
can define it locally and clearly. Makes adapting it for the next
generation much easier as you only have the single location to worry
about. (The normal argument for the macro, but in reverse.)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Read the Base of Stolen Memory for 915gm

2013-02-10 Thread Paul Menzel
Am Sonntag, den 10.02.2013, 19:38 + schrieb Chris Wilson:
> Reading the cspec pays dividends once again, as I found the 'Base of
> Stolen Memory' config register so that we can skip the fragile
> computation from Top of Usable Draw and use the real value provided by
> the BIOS.

Nice find. What is the expected functionality change as the code
beforehand was a no-op due to the `if 0`?

If testing is needed, how can this be tested?

> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c |9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 130d1db..7f1735c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -72,13 +72,10 @@ static unsigned long i915_stolen_to_physical(struct 
> drm_device *dev)
>   } else if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) {
>   /* Read Graphics Base of Stolen Memory directly */
>   pci_read_config_dword(pdev, 0xA4, &base);
> -#if 0
>   } else if (IS_GEN3(dev)) {
> - u8 val;
> - /* Stolen is immediately below Top of Low Usable DRAM */
> - pci_read_config_byte(pdev, 0x9c, &val);
> - base = val >> 3 << 27;
> - base -= dev_priv->mm.gtt->stolen_size;
> + /* Read D2:F0 Base of Stolen Memory directly */
> + pci_read_config_dword(dev->pdev, 0x5c, &base);

Should macros be defined for the register names?

> +#if 0
>   } else {
>   /* Stolen is immediately above Top of Memory */
>   base = max_low_pfn_mapped << PAGE_SHIFT;


Thanks,

Paul



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Read the Base of Stolen Memory for 915gm

2013-02-10 Thread Ben Widawsky
On Sun, Feb 10, 2013 at 07:38:13PM +, Chris Wilson wrote:
> Reading the cspec pays dividends once again, as I found the 'Base of
> Stolen Memory' config register so that we can skip the fragile
> computation from Top of Usable Draw and use the real value provided by
> the BIOS.
> 
> Signed-off-by: Chris Wilson 
Don't really want to dig out the doc, but I so prefer this to the
old way.
Acked-by: Ben Widawsky 

> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c |9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 130d1db..7f1735c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -72,13 +72,10 @@ static unsigned long i915_stolen_to_physical(struct 
> drm_device *dev)
>   } else if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) {
>   /* Read Graphics Base of Stolen Memory directly */
>   pci_read_config_dword(pdev, 0xA4, &base);
> -#if 0
>   } else if (IS_GEN3(dev)) {
> - u8 val;
> - /* Stolen is immediately below Top of Low Usable DRAM */
> - pci_read_config_byte(pdev, 0x9c, &val);
> - base = val >> 3 << 27;
> - base -= dev_priv->mm.gtt->stolen_size;
> + /* Read D2:F0 Base of Stolen Memory directly */
> + pci_read_config_dword(dev->pdev, 0x5c, &base);
> +#if 0
>   } else {
>   /* Stolen is immediately above Top of Memory */
>   base = max_low_pfn_mapped << PAGE_SHIFT;
> -- 
> 1.7.10.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


[Intel-gfx] [PATCH] drm/i915: Read the Base of Stolen Memory for 915gm

2013-02-10 Thread Chris Wilson
Reading the cspec pays dividends once again, as I found the 'Base of
Stolen Memory' config register so that we can skip the fragile
computation from Top of Usable Draw and use the real value provided by
the BIOS.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem_stolen.c |9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 130d1db..7f1735c 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -72,13 +72,10 @@ static unsigned long i915_stolen_to_physical(struct 
drm_device *dev)
} else if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) {
/* Read Graphics Base of Stolen Memory directly */
pci_read_config_dword(pdev, 0xA4, &base);
-#if 0
} else if (IS_GEN3(dev)) {
-   u8 val;
-   /* Stolen is immediately below Top of Low Usable DRAM */
-   pci_read_config_byte(pdev, 0x9c, &val);
-   base = val >> 3 << 27;
-   base -= dev_priv->mm.gtt->stolen_size;
+   /* Read D2:F0 Base of Stolen Memory directly */
+   pci_read_config_dword(dev->pdev, 0x5c, &base);
+#if 0
} else {
/* Stolen is immediately above Top of Memory */
base = max_low_pfn_mapped << PAGE_SHIFT;
-- 
1.7.10.4

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