[PATCH] drm/radeon: Add early unregister of firmware fb's

[Benjamin Herrenschmidt]

Without this, we attempt the handover too late, the firmware fb
might be accessing the chip simultaneously to us re-initializing
various parts of it, which might frighten babies or cause all sort
of nasty psychologic trauma to kitten.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
 drivers/gpu/drm/radeon/radeon_device.c |   18 ++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index cc1464b..9a3e6ba 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -594,6 +594,21 @@ static bool radeon_switcheroo_can_switch(struct pci_dev 
*pdev)
 }
 
 
+static void radeon_kick_out_firmware_fb(struct drm_device *ddev)
+{
+   struct apertures_struct *ap;
+   bool primary = false;
+
+   ap = alloc_apertures(1);
+   ap-ranges[0].base = drm_get_resource_start(ddev, 0);
+   ap-ranges[0].size = drm_get_resource_len(ddev, 0);
+
+#ifdef CONFIG_X86
+   primary = dev-pdev-resource[PCI_ROM_RESOURCE].flags  
IORESOURCE_ROM_SHADOW;
+#endif
+   remove_conflicting_framebuffers(ap, radeondrmfb, primary);
+}
+
 int radeon_device_init(struct radeon_device *rdev,
   struct drm_device *ddev,
   struct pci_dev *pdev,
@@ -633,6 +648,9 @@ int radeon_device_init(struct radeon_device *rdev,
init_waitqueue_head(rdev-irq.vblank_queue);
init_waitqueue_head(rdev-irq.idle_queue);
 
+   /* Get rid of things like offb */
+   radeon_kick_out_firmware_fb(ddev);
+
/* setup workqueue */
rdev-wq = create_workqueue(radeon);
if (rdev-wq == NULL)


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: Add early unregister of firmware fb's

[Benjamin Herrenschmidt]

On Mon, 2010-08-16 at 09:00 +0200, Rafał Miłecki wrote:
 2010/8/10 Benjamin Herrenschmidt b...@kernel.crashing.org:
  +#ifdef CONFIG_X86
  +   primary = dev-pdev-resource[PCI_ROM_RESOURCE].flags 
 IORESOURCE_ROM_SHADOW;
  +#endif
 
 It won't compile for CONFIG_X86, no dev.

Ah right, I've done more fixes, I'll send a new series soon.

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Revert 737a3bb9416ce2a7c7a4170852473a4fcc9c67e8 ?

[Benjamin Herrenschmidt]

On Wed, 2011-04-13 at 09:59 +0200, Gabriel Paubert wrote:
 
 Well, X is dead, or rather in an infinite ioctl loop as described
 above.
 IIRC, the display enters a power-down mode and there is nothing to
 see.

So basically the card crashed. There's about an infinite amount of
reasons why radeons do so, sometimes it has to do with them not liking
what you ate that day...

The only thing I can see that could be of use would be a bisect

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] Big Endian support for RV730 (Mesa r600)

[Benjamin Herrenschmidt]

On Tue, 2011-04-12 at 10:01 +0200, Cédric Cano wrote:
 Hi
 
 Here you are a patch that adds big endian support for rv730 in r600 
 classic mesa driver. The BE modifications are almost the same as the DRM 
 / DDX driver modifications 
 (http://lists.freedesktop.org/archives/dri-devel/2011-February/008151.html).
 
 I used the mesa-demos to test the driver status on big endian platform. 
 Nearly all demos renders the same as on Intel architecture. 
 Nevertheless, there are still some issues in glReadPixels (r600_blit) 
 with some formats. I can't figure out exactly what and when data must be 
 swapped (set_tex_resoures, set_render_target...). Review of the patch 
 would be greatly appreciated.
 
 It seems that r600g will be the default for Mesa 7.11 so I'll try to 
 enable big endian support for Gallium now.

Cool stuff !

I'll try to test that one of these days on various ppc's

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] Big Endian support for RV730 (Mesa r600)

[Benjamin Herrenschmidt]

On Wed, 2011-04-13 at 22:05 +1000, Benjamin Herrenschmidt wrote:
 On Tue, 2011-04-12 at 10:01 +0200, Cédric Cano wrote:
  Hi
  
  Here you are a patch that adds big endian support for rv730 in r600 
  classic mesa driver. The BE modifications are almost the same as the DRM 
  / DDX driver modifications 
  (http://lists.freedesktop.org/archives/dri-devel/2011-February/008151.html).
  
  I used the mesa-demos to test the driver status on big endian platform. 
  Nearly all demos renders the same as on Intel architecture. 
  Nevertheless, there are still some issues in glReadPixels (r600_blit) 
  with some formats. I can't figure out exactly what and when data must be 
  swapped (set_tex_resoures, set_render_target...). Review of the patch 
  would be greatly appreciated.
  
  It seems that r600g will be the default for Mesa 7.11 so I'll try to 
  enable big endian support for Gallium now.
 
 Cool stuff !
 
 I'll try to test that one of these days on various ppc's

BTW. I see you used some FSL embedded board. Do you have your PCIe MMIO
space above 32-bit ? Last I looked, there was a bunch of fixing needing
to be done, among others in the TTM, to make that work.

I had some preliminary patches but they bitrot... mostly the issue is to
make sure than a phys_addr_t is used instead of an unsigned long
whenever it tries to store the physical address of an object.

Ben.
 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] agp/uninorth: Fix lockups with radeon KMS and 1x.

[Benjamin Herrenschmidt]

On Thu, 2011-05-19 at 16:08 +0200, Michel Dänzer wrote:
 From: Michel Dänzer daen...@vmware.com
 
 This was based on a description by Ben Herrenschmidt:
 
  I've removed that SBA reset from the normal TLB invalidation path and
  left it only once after turning AGP on.
 
 About six months ago, he said:
 
  I did it a bit differently, but yeah, you get the idea. I'm doing a
  patch series so don't bother pushing things too hard yet.
 
 But I haven't seen anything from him about this since then, and people are
 regularly hitting these lockups, so here we are...
 
 Signed-off-by: Michel Dänzer daen...@vmware.com

Oops. I do have a pile of patches, but I never got something stable
enough and got distracted by more important stuff. Dave, please merge
this for now.

Acked-by: Benjamin Herrenschmidt b...@kernel.crashing.org

Thanks !

Cheers,
Ben.

 ---
  drivers/char/agp/uninorth-agp.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
 index 47c2218..55af723 100644
 --- a/drivers/char/agp/uninorth-agp.c
 +++ b/drivers/char/agp/uninorth-agp.c
 @@ -80,7 +80,7 @@ static void uninorth_tlbflush(struct agp_memory *mem)
  ctrl | UNI_N_CFG_GART_INVAL);
   pci_write_config_dword(agp_bridge-dev, UNI_N_CFG_GART_CTRL, ctrl);
  
 - if (uninorth_rev = 0x30) {
 + if (!mem  uninorth_rev = 0x30) {
   pci_write_config_dword(agp_bridge-dev, UNI_N_CFG_GART_CTRL,
  ctrl | UNI_N_CFG_GART_2xRESET);
   pci_write_config_dword(agp_bridge-dev, UNI_N_CFG_GART_CTRL,


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6] drm/radeon: Fix the definition of RADEON_BUF_SWAP_32BIT

[Benjamin Herrenschmidt]

(Note that this is duplicated under various other names such
as R600_BUF_SWAP_32BIT etc...). At least now all the definitions
agree.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---

(resent adding dri-devel to the CC list to hit patchwork)

 drivers/gpu/drm/radeon/radeon_reg.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_reg.h 
b/drivers/gpu/drm/radeon/radeon_reg.h
index ec93a75..0828add1 100644
--- a/drivers/gpu/drm/radeon/radeon_reg.h
+++ b/drivers/gpu/drm/radeon/radeon_reg.h
@@ -3293,7 +3293,7 @@
 #  define RADEON_RB_BUFSZ_MASK (0x3f  0)
 #  define RADEON_RB_BLKSZ_SHIFT8
 #  define RADEON_RB_BLKSZ_MASK (0x3f  8)
-#  define RADEON_BUF_SWAP_32BIT(1  17)
+#  define RADEON_BUF_SWAP_32BIT(2  16)
 #  define RADEON_MAX_FETCH_SHIFT   18
 #  define RADEON_MAX_FETCH_MASK(0x3  18)
 #  define RADEON_RB_NO_UPDATE  (1  27)



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/6] drm/radeon: Add a rmb() in IH processing

[Benjamin Herrenschmidt]

We should have a read memory barrier between reading the WPTR from
memory and reading ring entries based on that value (ie, we need to
ensure both loads are done in order by the CPU).

It could be argued that the MMIO reads in r600_ack_irq() might be
enough to get that barrier but I prefer keeping an explicit one just
in case.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
 
(resent adding dri-devel to the CC list to hit patchwork)

drivers/gpu/drm/radeon/r600.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 3c86b15..7e5c801 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -3312,6 +3312,9 @@ int r600_irq_process(struct radeon_device *rdev)
}
 
 restart_ih:
+   /* Order reading of wptr vs. reading of IH ring data */
+   wmb();
+
/* display interrupts */
r600_irq_ack(rdev);
 



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/6] drm/radeon: Do an MMIO read on interrupts when not uisng MSIs

[Benjamin Herrenschmidt]

When not using MSIs, there is no guarantee that DMA from the device
has been fully flushed to point where it's visible to the CPU when
taking an interrupt. To get this guarantee, we need to perform an
MMIO read from the device, which will flush all outstanding DMAs
from bridges between the device and the system.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---

 (resent adding dri-devel to the CC list to hit patchwork)

drivers/gpu/drm/radeon/r600.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 7e5c801..25b2dab 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -3300,6 +3300,10 @@ int r600_irq_process(struct radeon_device *rdev)
if (!rdev-ih.enabled || rdev-shutdown)
return IRQ_NONE;
 
+   /* No MSIs, need a dummy read to flush PCI DMAs */
+   if (!rdev-msi_enabled)
+   RREG32(IH_RB_WPTR);
+
wptr = r600_get_ih_wptr(rdev);
rptr = rdev-ih.rptr;
DRM_DEBUG(r600_irq_process start: rptr %d, wptr %d\n, rptr, wptr);



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/6] drm/radeon: Remove a bunch of useless _iomem casts

[Benjamin Herrenschmidt]

Just defining rdev-rmmio properly in the first place should do
the trick. In some cases, the cast were also complete dups as
the original variable was already of the right type.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---

(resent adding dri-devel to the CC list to hit patchwork)

 drivers/gpu/drm/radeon/radeon.h |   22 +++---
 drivers/gpu/drm/radeon/rs600.c  |2 +-
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index ef0e0e0..c92cf2c 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1174,7 +1174,7 @@ struct radeon_device {
/* Register mmio */
resource_size_t rmmio_base;
resource_size_t rmmio_size;
-   void*rmmio;
+   void __iomem*rmmio;
radeon_rreg_t   mc_rreg;
radeon_wreg_t   mc_wreg;
radeon_rreg_t   pll_rreg;
@@ -1251,20 +1251,20 @@ int radeon_gpu_wait_for_idle(struct radeon_device 
*rdev);
 static inline uint32_t r100_mm_rreg(struct radeon_device *rdev, uint32_t reg)
 {
if (reg  rdev-rmmio_size)
-   return readl(((void __iomem *)rdev-rmmio) + reg);
+   return readl((rdev-rmmio) + reg);
else {
-   writel(reg, ((void __iomem *)rdev-rmmio) + RADEON_MM_INDEX);
-   return readl(((void __iomem *)rdev-rmmio) + RADEON_MM_DATA);
+   writel(reg, (rdev-rmmio) + RADEON_MM_INDEX);
+   return readl((rdev-rmmio) + RADEON_MM_DATA);
}
 }
 
 static inline void r100_mm_wreg(struct radeon_device *rdev, uint32_t reg, 
uint32_t v)
 {
if (reg  rdev-rmmio_size)
-   writel(v, ((void __iomem *)rdev-rmmio) + reg);
+   writel(v, (rdev-rmmio) + reg);
else {
-   writel(reg, ((void __iomem *)rdev-rmmio) + RADEON_MM_INDEX);
-   writel(v, ((void __iomem *)rdev-rmmio) + RADEON_MM_DATA);
+   writel(reg, (rdev-rmmio) + RADEON_MM_INDEX);
+   writel(v, (rdev-rmmio) + RADEON_MM_DATA);
}
 }
 
@@ -1296,10 +1296,10 @@ static inline void r100_io_wreg(struct radeon_device 
*rdev, u32 reg, u32 v)
 /*
  * Registers read  write functions.
  */
-#define RREG8(reg) readb(((void __iomem *)rdev-rmmio) + (reg))
-#define WREG8(reg, v) writeb(v, ((void __iomem *)rdev-rmmio) + (reg))
-#define RREG16(reg) readw(((void __iomem *)rdev-rmmio) + (reg))
-#define WREG16(reg, v) writew(v, ((void __iomem *)rdev-rmmio) + (reg))
+#define RREG8(reg) readb((rdev-rmmio) + (reg))
+#define WREG8(reg, v) writeb(v, (rdev-rmmio) + (reg))
+#define RREG16(reg) readw((rdev-rmmio) + (reg))
+#define WREG16(reg, v) writew(v, (rdev-rmmio) + (reg))
 #define RREG32(reg) r100_mm_rreg(rdev, (reg))
 #define DREG32(reg) printk(KERN_INFO REGISTER:  #reg  : 0x%08X\n, 
r100_mm_rreg(rdev, (reg)))
 #define WREG32(reg, v) r100_mm_wreg(rdev, (reg), (v))
diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
index 6e3b11e..6779576 100644
--- a/drivers/gpu/drm/radeon/rs600.c
+++ b/drivers/gpu/drm/radeon/rs600.c
@@ -530,7 +530,7 @@ int rs600_gart_set_page(struct radeon_device *rdev, int i, 
uint64_t addr)
addr = addr  0xF000ULL;
addr |= R600_PTE_VALID | R600_PTE_SYSTEM | R600_PTE_SNOOPED;
addr |= R600_PTE_READABLE | R600_PTE_WRITEABLE;
-   writeq(addr, ((void __iomem *)ptr) + (i * 8));
+   writeq(addr, ptr + (i * 8));
return 0;
 }
 



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6] drm/radeon: Writeback endian fixes

[Benjamin Herrenschmidt]

The writeback ring pointer and IH ring pointer are read using le32_to_cpu
so we do not want the chip to byteswap them on big-endian.

We still want to byteswap the ring itself and the IBs, so we don't touch
that but we remove setting of the byteswap bits in CP_RB_RPTR_ADDR and
IH_CNTL.

In general, for things like that where we control all the accessors easily,
we are better off doing the swap in SW rather than HW. Paradoxally, it does
keep the code closer to x86 and avoid using poorly tested HW features.

I also changed the use of RADEON_ to R600_ in a couple of cases to be more
consistent with the surrounding code.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---

(resent adding dri-devel to the CC list to hit patchwork)

 drivers/gpu/drm/radeon/evergreen.c |3 ---
 drivers/gpu/drm/radeon/r600.c  |7 ---
 drivers/gpu/drm/radeon/r600_cp.c   |   23 +--
 3 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c 
b/drivers/gpu/drm/radeon/evergreen.c
index e8a5ffb..23cf089 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -1359,9 +1359,6 @@ int evergreen_cp_resume(struct radeon_device *rdev)
 
/* set the wb address wether it's enabled or not */
WREG32(CP_RB_RPTR_ADDR,
-#ifdef __BIG_ENDIAN
-  RB_RPTR_SWAP(2) |
-#endif
   ((rdev-wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET)  0xFFFC));
WREG32(CP_RB_RPTR_ADDR_HI, upper_32_bits(rdev-wb.gpu_addr + 
RADEON_WB_CP_RPTR_OFFSET)  0xFF);
WREG32(SCRATCH_ADDR, ((rdev-wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET)  
8)  0x);
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index f79d2cc..3c86b15 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2212,9 +2212,6 @@ int r600_cp_resume(struct radeon_device *rdev)
 
/* set the wb address whether it's enabled or not */
WREG32(CP_RB_RPTR_ADDR,
-#ifdef __BIG_ENDIAN
-  RB_RPTR_SWAP(2) |
-#endif
   ((rdev-wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET)  0xFFFC));
WREG32(CP_RB_RPTR_ADDR_HI, upper_32_bits(rdev-wb.gpu_addr + 
RADEON_WB_CP_RPTR_OFFSET)  0xFF);
WREG32(SCRATCH_ADDR, ((rdev-wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET)  
8)  0x);
@@ -2993,10 +2990,6 @@ int r600_irq_init(struct radeon_device *rdev)
/* RPTR_REARM only works if msi's are enabled */
if (rdev-msi_enabled)
ih_cntl |= RPTR_REARM;
-
-#ifdef __BIG_ENDIAN
-   ih_cntl |= IH_MC_SWAP(IH_MC_SWAP_32BIT);
-#endif
WREG32(IH_CNTL, ih_cntl);
 
/* force the active interrupt state to all disabled */
diff --git a/drivers/gpu/drm/radeon/r600_cp.c b/drivers/gpu/drm/radeon/r600_cp.c
index c3ab959..45fd592 100644
--- a/drivers/gpu/drm/radeon/r600_cp.c
+++ b/drivers/gpu/drm/radeon/r600_cp.c
@@ -1802,8 +1802,8 @@ static void r600_cp_init_ring_buffer(struct drm_device 
*dev,
/* Set ring buffer size */
 #ifdef __BIG_ENDIAN
RADEON_WRITE(R600_CP_RB_CNTL,
-RADEON_BUF_SWAP_32BIT |
-RADEON_RB_NO_UPDATE |
+R600_BUF_SWAP_32BIT |
+R600_RB_NO_UPDATE |
 (dev_priv-ring.rptr_update_l2qw  8) |
 dev_priv-ring.size_l2qw);
 #else
@@ -1820,15 +1820,15 @@ static void r600_cp_init_ring_buffer(struct drm_device 
*dev,
 
 #ifdef __BIG_ENDIAN
RADEON_WRITE(R600_CP_RB_CNTL,
-RADEON_BUF_SWAP_32BIT |
-RADEON_RB_NO_UPDATE |
-RADEON_RB_RPTR_WR_ENA |
+R600_BUF_SWAP_32BIT |
+R600_RB_NO_UPDATE |
+R600_RB_RPTR_WR_ENA |
 (dev_priv-ring.rptr_update_l2qw  8) |
 dev_priv-ring.size_l2qw);
 #else
RADEON_WRITE(R600_CP_RB_CNTL,
-RADEON_RB_NO_UPDATE |
-RADEON_RB_RPTR_WR_ENA |
+R600_RB_NO_UPDATE |
+R600_RB_RPTR_WR_ENA |
 (dev_priv-ring.rptr_update_l2qw  8) |
 dev_priv-ring.size_l2qw);
 #endif
@@ -1851,13 +1851,8 @@ static void r600_cp_init_ring_buffer(struct drm_device 
*dev,
- ((unsigned long) dev-sg-virtual)
+ dev_priv-gart_vm_start;
}
-   RADEON_WRITE(R600_CP_RB_RPTR_ADDR,
-#ifdef __BIG_ENDIAN
-(2  0) |
-#endif
-(rptr_addr  0xfffc));
-   RADEON_WRITE(R600_CP_RB_RPTR_ADDR_HI,
-upper_32_bits(rptr_addr));
+   RADEON_WRITE(R600_CP_RB_RPTR_ADDR, (rptr_addr  0xfffc));
+   RADEON_WRITE(R600_CP_RB_RPTR_ADDR_HI, upper_32_bits(rptr_addr));
 
 #ifdef __BIG_ENDIAN
RADEON_WRITE(R600_CP_RB_CNTL,



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http

Re: [PATCH 2/6] drm/radeon: ATOM Endian fix for atombios_crtc_program_pll()

[Benjamin Herrenschmidt]

On Wed, 2011-07-13 at 10:38 -0400, Alex Deucher wrote:

 case 6:
  -   args.v6.ulCrtcPclkFreq.ucCRTC = crtc_id;
  -   args.v6.ulCrtcPclkFreq.ulPixelClock = 
  cpu_to_le32(clock / 10);
  +   args.v6.ulDispEngClkFreq = cpu_to_le32(crtc_id  
  24 | clock / 10);
 
 For clarity (i can never remember op precedence), you might put:
 
 cpu_to_le32((crtc_id  24) | (clock / 10));

I can't either but I have a nice chart printed on my wall :-)

I can respin if you really want but the above is correct.

Cheers,
Ben.

 Other than that,
 
 Reviewed-by: Alex Deucher alexander.deuc...@amd.com
 
 
 args.v6.ucRefDiv = ref_div;
 args.v6.usFbDiv = cpu_to_le16(fb_div);
 args.v6.ulFbDivDecFrac = cpu_to_le32(frac_fb_div * 
  10);
 
 
 
 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm/radeon: Writeback endian fixes

[Benjamin Herrenschmidt]

On Wed, 2011-07-13 at 10:42 -0400, Alex Deucher wrote:
 On Wed, Jul 13, 2011 at 2:28 AM, Benjamin Herrenschmidt
 b...@kernel.crashing.org wrote:
  The writeback ring pointer and IH ring pointer are read using le32_to_cpu
  so we do not want the chip to byteswap them on big-endian.
 
  We still want to byteswap the ring itself and the IBs, so we don't touch
  that but we remove setting of the byteswap bits in CP_RB_RPTR_ADDR and
  IH_CNTL.
 
  In general, for things like that where we control all the accessors easily,
  we are better off doing the swap in SW rather than HW. Paradoxally, it does
  keep the code closer to x86 and avoid using poorly tested HW features.
 
  I also changed the use of RADEON_ to R600_ in a couple of cases to be more
  consistent with the surrounding code.
 
  Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
 
 evergreen.c and ni.c will need similar fixes.

evergreen.c -is- fixed in the patch :-) ni.c doesn't seem to set
swapping on the write back of the ring pointer (can you dbl check ?), it
only enables swapping on the ring itself which is correct as far as I
can tell.

Cheers,
Ben.

 Reviewed-by: Alex Deucher alexander.deuc...@amd.com
 
  ---
 
  (resent adding dri-devel to the CC list to hit patchwork)
 
   drivers/gpu/drm/radeon/evergreen.c |3 ---
   drivers/gpu/drm/radeon/r600.c  |7 ---
   drivers/gpu/drm/radeon/r600_cp.c   |   23 +--
   3 files changed, 9 insertions(+), 24 deletions(-)
 
  diff --git a/drivers/gpu/drm/radeon/evergreen.c 
  b/drivers/gpu/drm/radeon/evergreen.c
  index e8a5ffb..23cf089 100644
  --- a/drivers/gpu/drm/radeon/evergreen.c
  +++ b/drivers/gpu/drm/radeon/evergreen.c
  @@ -1359,9 +1359,6 @@ int evergreen_cp_resume(struct radeon_device *rdev)
 
 /* set the wb address wether it's enabled or not */
 WREG32(CP_RB_RPTR_ADDR,
  -#ifdef __BIG_ENDIAN
  -  RB_RPTR_SWAP(2) |
  -#endif
((rdev-wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET)  
  0xFFFC));
 WREG32(CP_RB_RPTR_ADDR_HI, upper_32_bits(rdev-wb.gpu_addr + 
  RADEON_WB_CP_RPTR_OFFSET)  0xFF);
 WREG32(SCRATCH_ADDR, ((rdev-wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) 
   8)  0x);
  diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
  index f79d2cc..3c86b15 100644
  --- a/drivers/gpu/drm/radeon/r600.c
  +++ b/drivers/gpu/drm/radeon/r600.c
  @@ -2212,9 +2212,6 @@ int r600_cp_resume(struct radeon_device *rdev)
 
 /* set the wb address whether it's enabled or not */
 WREG32(CP_RB_RPTR_ADDR,
  -#ifdef __BIG_ENDIAN
  -  RB_RPTR_SWAP(2) |
  -#endif
((rdev-wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET)  
  0xFFFC));
 WREG32(CP_RB_RPTR_ADDR_HI, upper_32_bits(rdev-wb.gpu_addr + 
  RADEON_WB_CP_RPTR_OFFSET)  0xFF);
 WREG32(SCRATCH_ADDR, ((rdev-wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) 
   8)  0x);
  @@ -2993,10 +2990,6 @@ int r600_irq_init(struct radeon_device *rdev)
 /* RPTR_REARM only works if msi's are enabled */
 if (rdev-msi_enabled)
 ih_cntl |= RPTR_REARM;
  -
  -#ifdef __BIG_ENDIAN
  -   ih_cntl |= IH_MC_SWAP(IH_MC_SWAP_32BIT);
  -#endif
 WREG32(IH_CNTL, ih_cntl);
 
 /* force the active interrupt state to all disabled */
  diff --git a/drivers/gpu/drm/radeon/r600_cp.c 
  b/drivers/gpu/drm/radeon/r600_cp.c
  index c3ab959..45fd592 100644
  --- a/drivers/gpu/drm/radeon/r600_cp.c
  +++ b/drivers/gpu/drm/radeon/r600_cp.c
  @@ -1802,8 +1802,8 @@ static void r600_cp_init_ring_buffer(struct 
  drm_device *dev,
 /* Set ring buffer size */
   #ifdef __BIG_ENDIAN
 RADEON_WRITE(R600_CP_RB_CNTL,
  -RADEON_BUF_SWAP_32BIT |
  -RADEON_RB_NO_UPDATE |
  +R600_BUF_SWAP_32BIT |
  +R600_RB_NO_UPDATE |
  (dev_priv-ring.rptr_update_l2qw  8) |
  dev_priv-ring.size_l2qw);
   #else
  @@ -1820,15 +1820,15 @@ static void r600_cp_init_ring_buffer(struct 
  drm_device *dev,
 
   #ifdef __BIG_ENDIAN
 RADEON_WRITE(R600_CP_RB_CNTL,
  -RADEON_BUF_SWAP_32BIT |
  -RADEON_RB_NO_UPDATE |
  -RADEON_RB_RPTR_WR_ENA |
  +R600_BUF_SWAP_32BIT |
  +R600_RB_NO_UPDATE |
  +R600_RB_RPTR_WR_ENA |
  (dev_priv-ring.rptr_update_l2qw  8) |
  dev_priv-ring.size_l2qw);
   #else
 RADEON_WRITE(R600_CP_RB_CNTL,
  -RADEON_RB_NO_UPDATE |
  -RADEON_RB_RPTR_WR_ENA |
  +R600_RB_NO_UPDATE |
  +R600_RB_RPTR_WR_ENA |
  (dev_priv-ring.rptr_update_l2qw  8) |
  dev_priv-ring.size_l2qw);
   #endif
  @@ -1851,13 +1851,8 @@ static void r600_cp_init_ring_buffer(struct 
  drm_device *dev

Re: [PATCH 4/6] drm/radeon: Add a rmb() in IH processing

[Benjamin Herrenschmidt]

On Wed, 2011-07-13 at 10:48 -0400, Alex Deucher wrote:
 On Wed, Jul 13, 2011 at 10:43 AM, Alex Deucher alexdeuc...@gmail.com wrote:
  On Wed, Jul 13, 2011 at 2:28 AM, Benjamin Herrenschmidt
  b...@kernel.crashing.org wrote:
  We should have a read memory barrier between reading the WPTR from
  memory and reading ring entries based on that value (ie, we need to
  ensure both loads are done in order by the CPU).
 
  It could be argued that the MMIO reads in r600_ack_irq() might be
  enough to get that barrier but I prefer keeping an explicit one just
  in case.
 
  Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
 
  Reviewed-by: Alex Deucher alexander.deuc...@amd.com
 
 evergreen.c will need a similar fix.

Ok. I can do that.

Cheers,
Ben.

 Alex
 
 
  ---
 
  (resent adding dri-devel to the CC list to hit patchwork)
 
  drivers/gpu/drm/radeon/r600.c |3 +++
   1 files changed, 3 insertions(+), 0 deletions(-)
 
  diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
  index 3c86b15..7e5c801 100644
  --- a/drivers/gpu/drm/radeon/r600.c
  +++ b/drivers/gpu/drm/radeon/r600.c
  @@ -3312,6 +3312,9 @@ int r600_irq_process(struct radeon_device *rdev)
 }
 
   restart_ih:
  +   /* Order reading of wptr vs. reading of IH ring data */
  +   wmb();
  +
 /* display interrupts */
 r600_irq_ack(rdev);
 
 
 
 
 
 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm/radeon: Writeback endian fixes

[Benjamin Herrenschmidt]

On Thu, 2011-07-14 at 17:19 +0200, Michel Dänzer wrote:
 On Mit, 2011-07-13 at 16:28 +1000, Benjamin Herrenschmidt wrote:
  The writeback ring pointer and IH ring pointer are read using le32_to_cpu
  so we do not want the chip to byteswap them on big-endian.
  
  We still want to byteswap the ring itself and the IBs, so we don't touch
  that but we remove setting of the byteswap bits in CP_RB_RPTR_ADDR and
  IH_CNTL.
  
  In general, for things like that where we control all the accessors easily,
  we are better off doing the swap in SW rather than HW. Paradoxally, it does
  keep the code closer to x86 and avoid using poorly tested HW features.
 
 Absolutely. Unfortunately, when I fixed the CP writeback code to use
 le32_to_cpu(), I didn't realize the code for some GPU families was
 already using HW swappers for this. 
 
 
  I also changed the use of RADEON_ to R600_ in a couple of cases to be more
  consistent with the surrounding code.
 
 That should probably be in a separate patch. Either way, though:

I thought about it and decided it was trivial enough not to bother
re-doing the patches.

Alex/Dave/whoever's in charge, feel free to apply the current batch,
I'll send further cleanups/fixes as separate patches, possibly not
before next week or so.

Cheers,
Ben.

 Reviewed-by: Michel Dänzer mic...@daenzer.net



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/6] drm/radeon: Add a rmb() in IH processing

[Benjamin Herrenschmidt]

On Fri, 2011-07-15 at 04:19 +, Matt Turner wrote:
 On Wed, Jul 13, 2011 at 6:28 AM, Benjamin Herrenschmidt
 b...@kernel.crashing.org wrote:
  We should have a read memory barrier between reading the WPTR from
  memory and reading ring entries based on that value (ie, we need to
  ensure both loads are done in order by the CPU).
 
  It could be argued that the MMIO reads in r600_ack_irq() might be
  enough to get that barrier but I prefer keeping an explicit one just
  in case.
 
  Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
  ---
 
  (resent adding dri-devel to the CC list to hit patchwork)
 
  drivers/gpu/drm/radeon/r600.c |3 +++
   1 files changed, 3 insertions(+), 0 deletions(-)
 
  diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
  index 3c86b15..7e5c801 100644
  --- a/drivers/gpu/drm/radeon/r600.c
  +++ b/drivers/gpu/drm/radeon/r600.c
  @@ -3312,6 +3312,9 @@ int r600_irq_process(struct radeon_device *rdev)
 }
 
   restart_ih:
  +   /* Order reading of wptr vs. reading of IH ring data */
  +   wmb();
  +
 /* display interrupts */
 r600_irq_ack(rdev);
 
 The subject line says rmb(), but this says wmb(). Just want to verify
 what you have is correct.

Nice spotting, it's a typo and should have been rmb(). I'll fix it and
respin.

Cheers,
Ben

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: linux-3.3-rc2 and radeon kms failure on ppc32 with Radeon X1650PRO pcie

[Benjamin Herrenschmidt]

On Wed, 2012-02-15 at 03:23 +0100, acrux wrote:
 On Sun, 12 Feb 2012 11:00:43 +0100
 Michel Dänzer mic...@daenzer.net wrote:
 
  On Sam, 2012-02-11 at 21:00 +0100, acrux wrote: 
   
   Just a curiosity, i've only two powerpc machines[1] equipped with PCIE
   videocards and both them are not able to boot with radeonkms.
   Modern PCI-E videocards are not recognized by the old linux framebuffer
   subsystem and they solely can be managed by the new KMS frame buffer
   that doesn't work properly on Power Architecture.
  
  That's too broad a statement, it works fine on other PowerPC machines
  (PowerMacs, some embedded boards).
  
 
 hi Michel,
 thanks a lot for your help, i really appreciate it.
 
 If you say they were tested on real Power Architecture boards with PCIE
 videocards thus it is reassuring... and i'm happy that you understand
 my previus assertion wasn't affected by malevolence or sarcasm. Indeed
 i'm also a bit troubled 'n frustrated thinking that next release of
 mesa 'll do extend use of llvm (that doesn't work properly on linuxppc
 and totally untested on linuxppc64)
 
 Btw, to sum up the list of Power Architecture machines with PCIE  that
 aim to be a desktop/workstation: Apple iMac G5 (iSight), Apple PowerMac
 Quad G5, YDL Powerstation 2x970MP and Acube Sam460ex . And the last
 two, on present evidence (my attempts), aren't able to boot up if
 bootkernel has kms enabled.

Which radeon card, kernel log please ? I've successfully booted various
reasonably modern radeons on these. I've also used gnome3 with GL
acceleration etc... on some of these. Granted that was a few month ago
so it's possible that something regressed.

  Furthermore the first tree ones can
 fallback to the legacy OpenFirmware framebuffer and safely get a
 console.
 
  It looks like there's a problem with accessing the PCIe device memory,
  and at this point it's not even 100% clear that this is due to a problem
  in the driver, as opposed to e.g. in the platform code.
  
 
 it could be the right problem and i've CC BenH that has a better global
 perspective.

Can you give me more data about the problem please ? It could be a
recent regression or some setup problem.

Also I've noticed on the PowerStation some issues where heavy DMA use by
a video card will eventually lock up the system. From what I can tell
this is an issue with the northbridge, though a Quad G5 with the same
bridge (tho not quite the same revision) doesn't show the problem. Could
be a configuration issue.

Cheers,
Ben.



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: linux-3.3-rc2 and radeon kms failure on ppc32 with Radeon X1650PRO pcie

[Benjamin Herrenschmidt]

On Wed, 2012-02-15 at 08:39 +0100, Michel Dänzer wrote:

   Btw, to sum up the list of Power Architecture machines with PCIE  that
   aim to be a desktop/workstation: Apple iMac G5 (iSight), Apple PowerMac
   Quad G5, YDL Powerstation 2x970MP and Acube Sam460ex . And the last
   two, on present evidence (my attempts), aren't able to boot up if
   bootkernel has kms enabled.
  
  Which radeon card, kernel log please ?
 
 See
 http://lists.freedesktop.org/archives/dri-devel/2012-February/018792.html 
 (start of this thread) and 
 http://lists.freedesktop.org/archives/dri-devel/2012-February/018791.html .

Ok, so for the 460EX, I am not surprised things aren't working with KMS
 DRI2... the 460 is not cache coherent, and this is not handled by TTM
as far as I can tell.

The second case with no firmware is a bit more surprising, looks like
something bad happened on the PCI express bus or the kernel tried to
access something that the card rejected (target abort or PCIe equivalent
most likely), thus triggering a PLB error . That could be investigated a
bit more.

Note that you say this is smoe kind of SAM460EX card... but it claims
to be a Canyonlands in the device-tree... is that expected or do we have
yet another case of a vendor claiming to be the eval board they based
their design upon and generally screwing up in a major way ? IE. does it
indeed work with an -identical- device-tree to a canyonland and no
patches added to the machine support at all ?

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: linux-3.3-rc2 and radeon kms failure on ppc32 with Radeon X1650PRO pcie

[Benjamin Herrenschmidt]

On Thu, 2012-02-16 at 08:50 +0100, Michel Dänzer wrote:
   The second case with no firmware is a bit more surprising, looks like
   something bad happened on the PCI express bus or the kernel tried to
   access something that the card rejected (target abort or PCIe
   equivalent most likely), thus triggering a PLB error . That could be
   investigated a bit more.
 
 AFAICT in both cases the immediate problem is the PLB error on first
 access to VRAM, indicating some kind of problem with ioremap_wc() or
 generally PCIe device memory access.

I think the problem here is more along the lines of 32-bit physical
memory and ttm screwing up the physical addresses before mapping the
vram object.

Tony (CC) has some patches to address that part (for use with a 476
which is cache coherent, we got evergreen working on that).

I think he hasn't yet polished the patches enough (ie fixed all the
drivers for the change in types) but basically, quite a few places in
there need to store physical addresses in resource_size_t instead of
long's.

  Anyway after nearly ten years, due a lack in resources, i'm sadly going
  to suspend the CRUX PPC project and my activism pro free
  software thus i'm unable to follow these debug. I'll hold on to me
  the only YDL Powerstation then from the next weeks i'll can only follow
  trying to help in debug [1] on this specific machine.  
  
  [1]http://lists.freedesktop.org/archives/dri-devel/2012-January/018575.html
 
 For that one, I'd try adding some more debugging output to
 radeon_get_bios() to find out which method it ends up using to retrieve
 the ROM contents, and why it doesn't look like it's an ATOM BIOS.

Is it an Apple card or an x86 card ? Apple cards don't have the BIOS in
the right place unfortunately.

Cheers,
Ben.

 Does the same card work in an x86 machine?
 
 
 -- 
 Earthling Michel Dänzer   |   http://www.amd.com
 Libre software enthusiast |  Debian, X and DRI developer
 
 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: linux-3.3-rc2 and radeon kms failure on ppc32 with Radeon X1650PRO pcie

[Benjamin Herrenschmidt]

On Thu, 2012-02-16 at 09:26 +0100, Michel Dänzer wrote:
   For that one, I'd try adding some more debugging output to
   radeon_get_bios() to find out which method it ends up using to
 retrieve
   the ROM contents, and why it doesn't look like it's an ATOM BIOS.
  
  Is it an Apple card or an x86 card ? Apple cards don't have the BIOS
 in
  the right place unfortunately.
 
 I don't think there ever were any Mac Editions of FireGL cards, and if
 it wasn't an x86 card, I'd expect different error messages. 

There is -one- Mac edition of r5xx actually :-) I have one of them...
It's PCIe and came as an option for the PCIe latest generation G5s. At
some point Alex gave me a BIOS ROM file that we could use to get the
ATOM stuff from, but we never quite sorted out distribution and so never
added mechanisms to use it from the driver.

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: WARNING: at drivers/gpu/drm/radeon/radeon_object.c:236

[Benjamin Herrenschmidt]

On Tue, 2012-03-27 at 20:21 -0400, Dave Jones wrote:

 
 Stops the warning, and there are no additional side-effects,
 so looks all good here.

Same.

 Tested-by: Dave Jones da...@redhat.com

Tested-by: Benjamin Herrenschmidt b...@kernel.crashing.org

 thanks,
 
   Dave


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

broken nouveau dependency on power supply

[Benjamin Herrenschmidt]

Hi folks !

With CONFIG_POWER_SUPPLY=m  nouveau built-in we get a build failure:

drivers/built-in.o: In function `.nouveau_pm_trigger':
(.text+0xa56e8): undefined reference to `.power_supply_is_system_supplied'

nouveau probably needs to depends on CONFIG_POWER_SUPPLY to force a module
build with the latter is =m

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] nouveau: Fix crash when pci_ram_rom() returns a size of 0

[Benjamin Herrenschmidt]

From b15b244d6e6e20964bd4b85306722cb60c3c0809 Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt b...@kernel.crashing.org
Date: Mon, 2 Apr 2012 13:28:18 +1000
Subject: 

Under some circumstances, pci_map_rom() can return a valid mapping
but a size of 0 (if it cannot find an image in the header).

This causes nouveau to try to kmalloc() a 0 sized pointer and
dereference it, which crashes.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---

Please send to Linus asap, without this, it crashes on the G5

 drivers/gpu/drm/nouveau/nouveau_bios.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c 
b/drivers/gpu/drm/nouveau/nouveau_bios.c
index 637afe7..80963d0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bios.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
@@ -177,14 +177,15 @@ bios_shadow_pci(struct nvbios *bios)
 
if (!pci_enable_rom(pdev)) {
void __iomem *rom = pci_map_rom(pdev, length);
-   if (rom) {
+   if (rom  length) {
bios-data = kmalloc(length, GFP_KERNEL);
if (bios-data) {
memcpy_fromio(bios-data, rom, length);
bios-length = length;
}
-   pci_unmap_rom(pdev, rom);
}
+   if (rom)
+   pci_unmap_rom(pdev, rom);
 
pci_disable_rom(pdev);
}
-- 
1.7.9.5



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] nouveau/bios: Fix tracking of BIOS image data

[Benjamin Herrenschmidt]

The code tries various methods for retreiving the BIOS data. However
it doesn't clear the bios-data pointer between the iterations.

In some cases, the shadow() method will fail and not update bios-data
at all, which will cause us to score the old data and incorrectly
attribute that score to the new method. This can cause double frees
later when disposing of the unused data.

Additionally, we were not freeing the data for methods that fail the
score test (we only freed when a best is superseeded, not when the
new method has a lower score than the exising best). Fix that as well.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---

This was found by code inspection while chasing a different bug,
I do not hit the problem path on my machine.

 drivers/gpu/drm/nouveau/nouveau_bios.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c 
b/drivers/gpu/drm/nouveau/nouveau_bios.c
index 80963d0..1947d61 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bios.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
@@ -273,6 +273,7 @@ bios_shadow(struct drm_device *dev)
mthd-score = score_vbios(bios, mthd-rw);
mthd-size = bios-length;
mthd-data = bios-data;
+   bios-data = NULL;
} while (mthd-score != 3  (++mthd)-shadow);
 
mthd = shadow_methods;
@@ -281,7 +282,8 @@ bios_shadow(struct drm_device *dev)
if (mthd-score  best-score) {
kfree(best-data);
best = mthd;
-   }
+   } else
+   kfree(mthd-data);
} while ((++mthd)-shadow);
 
if (best-score) {
-- 
1.7.9.5



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: broken nouveau dependency on power supply

[Benjamin Herrenschmidt]

On Mon, 2012-04-02 at 11:06 +1000, Benjamin Herrenschmidt wrote:
 Hi folks !
 
 With CONFIG_POWER_SUPPLY=m  nouveau built-in we get a build failure:
 
 drivers/built-in.o: In function `.nouveau_pm_trigger':
 (.text+0xa56e8): undefined reference to `.power_supply_is_system_supplied'
 
 nouveau probably needs to depends on CONFIG_POWER_SUPPLY to force a module
 build with the latter is =m

Ok, not that trivial...

The problem is more like POWER_SUPPLY should be a bool, not a tristate.

If you think about it: you don't want things like nouveau to depend on a
random subsystem like that, people will never get it. In fact,
POWER_SUPPLY provides empty inline stubs when not enabled, so that's
really designed to not have depends...

However that -cannot- work if POWER_SUPPLY is modular and the drivers
who use it are not. The only fixes here that make sense I can think of
that don't also involve Kconfig horrors are:

 - Ugly: in power_supply.h, use the extern variant if

defined(CONFIG_POWER_SUPPLY) ||
 (defined(CONFIG_POWER_SUPPLY_MODULE)  defined(MODULE))

IE. use the stub if power supply is a module and what is being built is
built-in. Of course that's not only ugly, it somewhat sucks from a user
perspective as the subsystem now exists but can't be used by some
drivers...

 - Better: Just make the bloody thing a bool :-) The power supply
framework itself is small enough, just make it a boolean option and
avoid the problem entirely. The actual power supply sub drivers can
remain modular of course.

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: broken nouveau dependency on power supply

[Benjamin Herrenschmidt]

On Mon, 2012-04-02 at 05:00 -0400, David Airlie wrote:
   - Better: Just make the bloody thing a bool :-) The power supply
  framework itself is small enough, just make it a boolean option and
  avoid the problem entirely. The actual power supply sub drivers can
  remain modular of course.
 
 We can just do select POWER_SUPPLY.

Well, select'ing otherwise user configurable options was still frowned
on last we discussed that... and it makes the whole inline stubs in
power_supply.h totally pointless :-)

 Yes it reduces the option range for some stupid corner case but really
 I don't care, removing features from the kernel that a driver depends
 on is just leading to insane state combination and QA problems.

Well, the power supply stuff only works if you have a backend for it,
which not all platforms are, and it's fairly safe to assume AC whenever
it's not actually supported.

(Talking of which, we should be able to do a PowerBook backend
reasonably easily).

I don't care which solution you guys end up choosing though, just fix
it :-)

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Oops with Radeon/Uninorth on Maple

[Benjamin Herrenschmidt]

On Thu, 2012-05-24 at 10:18 +0400, Dmitry Eremin-Solenikov wrote:
 Hello, colleagues,
 
 I'm trying to enable an AGP slot (again) on my Maple board (dual 
 PPC970FX board, with CPC925 (U3H) north bridge).
 
   For now I'm stuck with a problem: I use radeon card, drm-radeon driver 
 with KMS.
 
 If I force drm-radeon to think about a card as about PCI card (by 
 commenting corresponding lines in drm_radeon_kms.c), everything works, I 
 get framebuffer, working X11, etc. If I enable agpgart-uninorth driver
 and RADEON_IS_AGP flag in drm driver, I get an Oops early during the 
 bootstrap. Relevant part of the log (I can send full dmesg of normal 
 bootstrap or this oops on request, if that would help).

Machine Check probably means that there's a HW configuration issue,
possibly something missing in the initialization of the AGP hardware to
make it actually work.

Cheers,
Ben.

 [2.820647] Linux agpgart interface v0.103
 [2.824909] agpgart-uninorth :f0:0b.0: Apple U3H chipset
 [2.830668] agpgart-uninorth :f0:0b.0: Found device u3, rev 35
 [2.843611] agpgart-uninorth :f0:0b.0: configuring for size idx: 64
 [2.850638] agpgart-uninorth :f0:0b.0: AGP aperture is 256M @ 0x0
 [2.857646] [drm] Initialized drm 1.1.0 20060810
 [2.862567] [drm] radeon defaulting to kernel modesetting.
 [2.868091] [drm] radeon kernel modesetting enabled.
 [2.873222] radeon :f0:10.0: enabling device ( - 0003)
 [2.880311] radeon :f0:10.0: enabling bus mastering
 [2.885591] [drm] initializing kernel modesetting (RV350 
 0x1002:0x4152 0x18BC:0x0416).
 [2.893629] [drm] register mmio base: 0xD002
 [2.898260] [drm] register mmio size: 65536
 [2.947112] [drm] GPU not posted. posting now...
 [3.051033] agpgart-uninorth :f0:0b.0: putting AGP V3 device into 
 8x mode
 [3.058197] radeon :f0:10.0: putting AGP V3 device into 8x mode
 [3.064666] radeon :f0:10.0: GTT: 256M 0x - 0x0FFF
 [3.070864] [drm] Generation 2 PCI interface, using max accessible memory
 [3.077672] radeon :f0:10.0: VRAM: 128M 0xC000 - 
 0xC7FF (128M used)
 [3.086487] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
 [3.093126] [drm] Driver supports precise vblank timestamp query.
 [3.099291] [drm] radeon: irq initialized.
 [3.103404] [drm] Detected VRAM RAM=128M, BAR=128M
 [3.108214] [drm] RAM width 128bits DDR
 [3.112263] [TTM] Zone  kernel: Available graphics memory: 496682 kiB
 [3.118732] [TTM] Initializing pool allocator
 [3.123346] [drm] radeon: 128M of VRAM memory ready
 [3.128256] [drm] radeon: 256M of GTT memory ready.
 [3.133295] [drm] radeon: ib pool ready.
 [3.137708] [drm] radeon: 1 quad pipes, 1 Z pipes initialized.
 [3.144018] radeon :f0:10.0: WB disabled
 [3.148326] [drm] fence driver on ring 0 use gpu addr 0x and 
 cpu addr 0xd0066000
 [3.157474] [drm] Loading R300 Microcode
 [3.162480] [drm] radeon: ring at 0x1000
 [3.167569] [drm] ring test succeeded in 0 usecs
 cpu 0x0: Vector: 200 (Machine Check) at [c0d63aa0]
  pc: c00cc07c: .trace_hardirqs_on_caller+0x6c/0x190
  lr: c00152f4: .cpu_idle+0x1a4/0x220
  sp: c0d63d20
 msr: 90009032
current = 0xc0c4db30
paca= 0xc000   softe: 0irq_happened: 0x01
  pid   = 0, comm = swapper/0
 enter ? for help
 [c0d63db0] c00152f4 .cpu_idle+0x1a4/0x220
 [c0d63e50] c0008fb8 .rest_init+0xe8/0x110
 [c0d63ee0] c0ba2998 .start_kernel+0x3e4/0x408
 [c0d63f90] c0007558 .start_here_common+0x20/0x48
 0:mon x
 [  843.783295] Oops: Machine check, sig: 7 [#1]
 [  843.787589] SMP NR_CPUS=4 Maple
 [  843.790768] Modules linked in:
 [  843.793855] NIP: c00cc07c LR: c00152f4 CTR: 
 c0023eac
 [  843.800920] REGS: c0d63aa0 TRAP: 0200   Not tainted  (3.4.0+)
 [  843.807376] MSR: 90009032 SF,HV,EE,ME,IR,DR,RI  CR: 
 2422  XER: 0006
 [  843.815412] SOFTE: 0
 [  843.817607] TASK = c0c4db30[0] 'swapper/0' THREAD: 
 c0d6 CPU: 0
 [  843.825035] GPR00:  c0d63d20 c0d63280 
 c00152f4
 [  843.833169] GPR04:  c0099d10 0001 
 0002
 [  843.841302] GPR08: 0100 c0e828e8 0140 
 
 [  843.849436] GPR12: 4482 c000  
 
 [  843.857570] GPR16: 00ff8750 00cdc890 010001e0 
 
 [  843.865702] GPR20:   1dcd6500 
 
 [  843.873835] GPR24:  00ec7b00 90009032 
 c0d7b178
 [  843.881979] GPR28: c0d7b278 0008 c0c970f8 
 c00152f4
 [  

[PATCH 4/6] drm/cirrus: Proper support for depth 15 and 16

[Benjamin Herrenschmidt]

We were configuring SR7 very strangely for 16bpp and didn't properly
differenciate between depth 15 and 16. This fixes it (and both
appear to work at least on ppc)

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
 drivers/gpu/drm/cirrus/cirrus_mode.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c 
b/drivers/gpu/drm/cirrus/cirrus_mode.c
index e3d2dc0..1566853 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -269,13 +269,14 @@ static int cirrus_crtc_mode_set(struct drm_crtc *crtc,
sr07 = RREG8(SEQ_DATA);
sr07 = 0xe0;
hdr = 0;
+
switch (crtc-fb-bits_per_pixel) {
case 8:
sr07 |= 0x11;
break;
case 16:
-   sr07 |= 0xc1;
-   hdr = 0xc0;
+   sr07 |= 0x17;
+   hdr = (crtc-fb-depth == 16) ? 0xc1 : 0xc0;
break;
case 24:
sr07 |= 0x15;


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6] drm/cirrus: Workaround for HDR access

[Benjamin Herrenschmidt]

Qemu has an odd behaviour with the access to HDR (could be a qemu
bug, I'm investigating separately, but it affects current qemu's
so we should try to work around it).

Basically the internal counter that counts the reads of the 3c6
register in order to give you access to the HDR on the 5th access
is initialized in such a way that it overflows if you start doing
the 4-reads sequence right after reset, and thus fails to pickup
the subsequent write.

We work around this by doing a write before the 4 reads, which
resets the counter properly.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
 drivers/gpu/drm/cirrus/cirrus_drv.h |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h 
b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 64ea597..b5685d3 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -74,6 +74,7 @@
 
 #define WREG_HDR(v)\
do {\
+   WREG8(VGA_DAC_MASK, v); /* reset counter */ \
RREG8(VGA_DAC_MASK);\
RREG8(VGA_DAC_MASK);\
RREG8(VGA_DAC_MASK);\


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/6] drm/fb: Fix depth 15 support in drm_fb_helper.c

[Benjamin Herrenschmidt]

fbset can pass var-bits_per_pixel = 15 when doing fbset -depth 15,
so we need to correct that to bpp 16 / depth 15.

Additionally, we make it possible to pass 15 as an argument to
drm_fb_helper_single_fb_probe() which will similarily select
a bpp of 15 and a depth of 15.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
 drivers/gpu/drm/drm_fb_helper.c |   17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f546d1e..da6873c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -571,6 +571,14 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
}
 
switch (var-bits_per_pixel) {
+   case 15:
+   /*
+* There is no such thing as a packed 15bpp,
+* so in this case, assume 16bpp, depth 15
+*/
+   depth = 15;
+   var-bits_per_pixel = 16;
+   break;
case 16:
depth = (var-green.length == 6) ? 16 : 15;
break;
@@ -722,10 +730,15 @@ int drm_fb_helper_single_fb_probe(struct drm_fb_helper 
*fb_helper,
sizes.fb_width = (unsigned)-1;
sizes.fb_height = (unsigned)-1;
 
-   /* if driver picks 8 or 16 by default use that
-  for both depth/bpp */
+   /*
+* If driver picks 8 or 16 by default use that
+* for both depth/bpp, however convert 15 to bpp 16
+* depth 15
+*/
if (preferred_bpp != sizes.surface_bpp) {
sizes.surface_depth = sizes.surface_bpp = preferred_bpp;
+   if (preferred_bpp == 15)
+   sizes.surface_bpp = 16;
}
/* first up get a count of crtcs now in use and new min/maxes 
width/heights */
for (i = 0; i  fb_helper-connector_count; i++) {



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/6] drm/cirrus: Check for pitch limits rather than bpp limits

[Benjamin Herrenschmidt]

The real HW limit that prevents from using 32bpp is a pitch
limit of 4095 bytes. 32bpp is otherwise supported and works.

This fixes the checks in the code to check the right thing
(so that a userspace request to set a mode with a supported
bpp but a too large pitch will fail appropriately).

Additionally, we make the fbdev code try reducing the bpp if
it hits the pitch limit in order to improve the chances of
displaying a console at boot if the default or requested mode
is too large to fit.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
 drivers/gpu/drm/cirrus/cirrus_fbdev.c |   28 +---
 drivers/gpu/drm/cirrus/cirrus_main.c  |4 ++--
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c 
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 9a276a5..1345215 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -109,9 +109,9 @@ static int cirrusfb_create_object(struct cirrus_fbdev 
*afbdev,
 
int ret = 0;
drm_fb_get_bpp_depth(mode_cmd-pixel_format, depth, bpp);
-
-   if (bpp  24)
+   if (mode_cmd-pitches[0] = 4096)
return -EINVAL;
+
size = mode_cmd-pitches[0] * mode_cmd-height;
ret = cirrus_gem_create(dev, size, true, gobj);
if (ret)
@@ -137,7 +137,29 @@ static int cirrusfb_create(struct cirrus_fbdev *gfbdev,
 
mode_cmd.width = sizes-surface_width;
mode_cmd.height = sizes-surface_height;
-   mode_cmd.pitches[0] = mode_cmd.width * ((sizes-surface_bpp + 7) / 8);
+
+   /* Reduce bpp to fit pitch constraints if necessary */
+   for (;;) {
+   mode_cmd.pitches[0] = mode_cmd.width * ((sizes-surface_bpp + 
7) / 8);
+   if (mode_cmd.pitches[0]  4096)
+   break;
+   switch(sizes-surface_bpp) {
+   case 32:
+   sizes-surface_bpp = 24;
+   break;
+   case 24:
+   sizes-surface_bpp = 16;
+   break;
+   case 16:
+   sizes-surface_bpp = 8;
+   break;
+   default:
+   return -ENOMEM;
+   }
+   DRM_INFO(Selected mode has a too high pitch, reducing to %d 
bpp\n,
+sizes-surface_bpp);
+   }
+
mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes-surface_bpp,
  sizes-surface_depth);
size = mode_cmd.pitches[0] * mode_cmd.height;
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c 
b/drivers/gpu/drm/cirrus/cirrus_main.c
index e3c1225..ac1a5e5 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -64,8 +64,8 @@ cirrus_user_framebuffer_create(struct drm_device *dev,
u32 bpp, depth;
 
drm_fb_get_bpp_depth(mode_cmd-pixel_format, depth, bpp);
-   /* cirrus can't handle  24bpp framebuffers at all */
-   if (bpp  24)
+   /* cirrus can't handle pitch = 4096  */
+   if (mode_cmd-pitches[0] = 4096)
return ERR_PTR(-EINVAL);
 
obj = drm_gem_object_lookup(dev, filp, mode_cmd-handles[0]);


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6] drm/cirrus: Retrieve default mode from the device-tree if any

[Benjamin Herrenschmidt]

On the pseries machine type, qemu puts a default mode in the
device-tree based on the user request (-g option) which the
firmware uses to setup the boot screen.

Currently cirrusdrmfb ignores this and always ends up using
1280x1024. This adds support for retrieving this information
and using it to set the default mode and depth.

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
 drivers/gpu/drm/cirrus/cirrus_fbdev.c |3 +--
 drivers/gpu/drm/cirrus/cirrus_mode.c  |   37 +
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c 
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 1345215..b0e4080 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -296,9 +296,8 @@ int cirrus_fbdev_init(struct cirrus_device *cdev)
 {
struct cirrus_fbdev *gfbdev;
int ret;
-   int bpp_sel = 24;
+   int bpp_sel = cdev-dev-mode_config.preferred_depth;
 
-   /*bpp_sel = 8;*/
gfbdev = kzalloc(sizeof(struct cirrus_fbdev), GFP_KERNEL);
if (!gfbdev)
return -ENOMEM;
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c 
b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 1566853..1ba73b0 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -19,6 +19,8 @@
 #include drm_crtc_helper.h
 
 #include video/cirrus.h
+#include linux/pci.h
+#include linux/of.h
 
 #include cirrus_drv.h
 
@@ -498,15 +500,25 @@ int cirrus_vga_get_modes(struct drm_connector *connector)
 {
int count = 0;
 
+#ifdef CONFIG_OF
+   /*
+* Add qemu default mode as preferred on machines where it's
+* indicated in the device-tree
+*/
+   if (of_chosen) {
+   u32 width, height;
+
+   if (of_property_read_u32(of_chosen, qemu,graphic-width, 
width) == 0 
+   of_property_read_u32(of_chosen, qemu,graphic-height, 
height) == 0)
+   count += drm_add_modes_noedid(connector, width, height, 
true);
+   }
+#endif /* CONFIG_OF */
+
/* Just add a static list of modes */
count += drm_add_modes_noedid(connector, 640, 480, false);
count += drm_add_modes_noedid(connector, 800, 600, false);
count += drm_add_modes_noedid(connector, 1024, 768, false);
count += drm_add_modes_noedid(connector, 1280, 1024, false);
-   drm_add_modes_noedid(connector, 640, 480);
-   drm_add_modes_noedid(connector, 800, 600);
-   drm_add_modes_noedid(connector, 1024, 768);
-   drm_add_modes_noedid(connector, 1280, 1024);
 
return count;
 }
@@ -597,6 +609,23 @@ int cirrus_modeset_init(struct cirrus_device *cdev)
 
cdev-dev-mode_config.fb_base = cdev-mc.vram_base;
cdev-dev-mode_config.preferred_depth = 24;
+
+#ifdef CONFIG_OF
+   /*
+* Add qemu default depth as preferred on machines where it's
+* indicated in the device-tree
+*/
+   if (of_chosen) {
+   u32 depth;
+
+   if (!of_property_read_u32(of_chosen, qemu,graphic-depth, 
depth)) {
+   if (depth == 8 || depth == 15 || depth == 16 ||
+   depth == 24 || depth == 32)
+   cdev-dev-mode_config.preferred_depth = depth;
+   }
+   }
+#endif /* CONFIG_OF */
+
/* don't prefer a shadow on virt GPU */
cdev-dev-mode_config.prefer_shadow = 0;
 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/6] drm/fb: Enable choosing a preferred noedid mode

[Benjamin Herrenschmidt]

This adds a preferred argument to drm_add_modes_noedid() which
allow drivers such as cirrusdrmfb to select a preferred mode based
on firmware configuration

Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
 drivers/gpu/drm/cirrus/cirrus_mode.c |8 +++-
 drivers/gpu/drm/drm_crtc_helper.c|2 +-
 drivers/gpu/drm/drm_edid.c   |7 ++-
 include/drm/drm_crtc.h   |2 +-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c 
b/drivers/gpu/drm/cirrus/cirrus_mode.c
index a44d31a..e3d2dc0 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -495,13 +495,19 @@ static struct drm_encoder *cirrus_encoder_init(struct 
drm_device *dev)
 
 int cirrus_vga_get_modes(struct drm_connector *connector)
 {
+   int count = 0;
+
/* Just add a static list of modes */
+   count += drm_add_modes_noedid(connector, 640, 480, false);
+   count += drm_add_modes_noedid(connector, 800, 600, false);
+   count += drm_add_modes_noedid(connector, 1024, 768, false);
+   count += drm_add_modes_noedid(connector, 1280, 1024, false);
drm_add_modes_noedid(connector, 640, 480);
drm_add_modes_noedid(connector, 800, 600);
drm_add_modes_noedid(connector, 1024, 768);
drm_add_modes_noedid(connector, 1280, 1024);
 
-   return 4;
+   return count;
 }
 
 static int cirrus_vga_mode_valid(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index 3252e70..2b8612f 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -126,7 +126,7 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,
count = (*connector_funcs-get_modes)(connector);
 
if (count == 0  connector-status == connector_status_connected)
-   count = drm_add_modes_noedid(connector, 1024, 768);
+   count = drm_add_modes_noedid(connector, 1024, 768, false);
if (count == 0)
goto prune;
 
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a8743c3..38cab74 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1945,7 +1945,7 @@ EXPORT_SYMBOL(drm_add_edid_modes);
  * Return number of modes added or 0 if we couldn't find any.
  */
 int drm_add_modes_noedid(struct drm_connector *connector,
-   int hdisplay, int vdisplay)
+int hdisplay, int vdisplay, bool preferred)
 {
int i, count, num_modes = 0;
struct drm_display_mode *mode;
@@ -1973,6 +1973,11 @@ int drm_add_modes_noedid(struct drm_connector *connector,
continue;
mode = drm_mode_duplicate(dev, ptr);
if (mode) {
+   if (preferred  ptr-hdisplay == hdisplay 
+   ptr-vdisplay == vdisplay) {
+   mode-type |= DRM_MODE_TYPE_PREFERRED;
+   preferred = false;
+   }
drm_mode_probed_add(connector, mode);
num_modes++;
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index a1a0386..e8e94b7 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1033,7 +1033,7 @@ extern struct drm_display_mode 
*drm_gtf_mode_complex(struct drm_device *dev,
bool interlaced, int margins, int GTF_M,
int GTF_2C, int GTF_K, int GTF_2J);
 extern int drm_add_modes_noedid(struct drm_connector *connector,
-   int hdisplay, int vdisplay);
+   int hdisplay, int vdisplay, bool preferred);
 
 extern int drm_edid_header_is_valid(const u8 *raw_edid);
 extern bool drm_edid_block_valid(u8 *raw_edid, int block);


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2 3/3] ppc64: implemented pcibios_get_speed_cap_mask

[Benjamin Herrenschmidt]

On Wed, 2013-03-20 at 02:24 -0300, Lucas Kannebley Tavares wrote:
 Implementation of a architecture-specific pcibios_get_speed_cap_mask.
 This implementation detects bus capabilities based on OF
 ibm,pcie-link-speed-stats property.

The problem with your approach is that it's not a runtime detection...

If the pseries machine is compiled into the kernel binary, it will
override pcibios_get_speed_cap_mask() using the device-tree, regardless
of whether the machine is currently booted on a pseries machine or not.

This wouldn't be a big problem if the pseries
pcibios_get_speed_cap_mask() was capable of doing a fallback to the
generic one if the device-tree property is absent but that isn't the
case.

I think what you need to do is:

  - Make it so the generic one can be called by the override. This can
look a bit tricky but it's better that way. One way to do it is to have
the actual implementation be in a __pci_* variant called by the weak
pcibios_* variant

  - Move the powerpc on to arch/powerpc/kernel/pci-common.c and make
it call a ppc_md.pcibios_get_speed_cap_mask(). If the hook is absent
(NULL), make it call the generic one

  - pseries can then populate the hook in ppc_md. with its custom
variant.

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2 1/3] pci: added pcie_get_speed_cap_mask function

[Benjamin Herrenschmidt]

On Tue, 2013-03-26 at 12:39 -0600, Bjorn Helgaas wrote:
 But we also know pdev is a PCIe device, and I think a PCIe device on a
 root bus must be a Root Complex Integrated Endpoint (PCIe spec sec
 1.3.2.3).  Such a device does not have a link at all, so there's no
 point in fiddling with its link speed.

This is where our IBM hypervisor makes things murky. It doesn't expose
the PCIe parents (basically somewhat makes PCIe look like PCI except we
still have the PCIe caps on the child devices, just no access to the
parent device).

It's garbage but can't be fixed (would break AIX :-)

However we might be able to populate the bus-max_bus_speed from some
architecture specific quirk and have radeon use that.

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv5 0/2] Speed Cap fixes for ppc64

[Benjamin Herrenschmidt]

On Fri, 2013-05-03 at 19:43 -0300, Kleber Sacilotto de Souza wrote:

 This patch series does:
   1. max_bus_speed is used to set the device to gen2 speeds
   2. on power there's no longer a conflict between the pseries call and other
 architectures, because the overwrite is done via a ppc_md hook
   3. radeon is using bus-max_bus_speed instead of drm_pcie_get_speed_cap_mask
 for gen2 capability detection
 
 The first patch consists of some architecture changes, such as adding a hook 
 on
 powerpc for pci_root_bridge_prepare, so that pseries will initialize it to a
 function, while all other architectures get a NULL pointer. So that whenever
 pci_create_root_bus is called, we'll get max_bus_speed properly setup from
 OpenFirmware.
 
 The second patch consists of simple radeon changes not to call
 drm_get_pcie_speed_cap_mask anymore. I assume that on x86 machines,
 the max_bus_speed property will be properly set already.

So I'm ok with the approach now and I might even put the powerpc patch
in for 3.10 since arguably we are fixing a nasty bug (uninitialized
max_bus_speed).

David, what's your feeling about the radeon change ? It would be nice if
that could go in soon for various distro targets :-) On the other hand
I'm not going to be pushy if you are not comfortable with it.

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Fixing nouveau for 4k PAGE_SIZE

[Benjamin Herrenschmidt]

Hi folks !

So I've been trying to figure out what it would take to make
nouveau work properly on architectures where PAGE_SIZE isn't
4k such as most ppc64's. An initial patch from Dave fixed a
bogon in nv41.c nv41_vm_map_sg() which was trying to handle
the case at that low level, but this isn't enough, and after
a bit of digging, I also think that's not the right approach:

So, from what I can tell, subdev/vm/base.c is not clean vs. PAGE_SIZE
in a number of places unless I'm mistaken. For example, in
nouveau_vm_map_sg_table(), something like that:

sglen = sg_dma_len(sg)  PAGE_SHIFT;

end = pte + sglen;

Seems to imply an assumption here that the pte is in multiple of
PAGE_SHIFT, but afaik, it's not. So further down, we do:

for (m = 0; m  len; m++) {
dma_addr_t addr = sg_dma_address(sg) + (m  
PAGE_SHIFT);

vmm-map_sg(vma, pgt, mem, pte, 1, addr);  
 
num--;
pte++;

But in fact, inside vmm-map_sg, with the current code, we will have
incremented pte by more than 1 ... so we basically lose track here.

if (num == 0)
goto finish;
}
if (unlikely(end = max)) {
pde++;
pte = 0;
}

We need to similarly make sure we don't end up crossing accross two
pde's here, ie, that our 64k page isn't mapped at a non-multiple of
64k in the card space, where is that enforced ? (It might be, I don't
know). If it is, then we need to recalc the pde on each sub page.

So I'm thinking the right fix is to remove the inner loop in nv41.c
nv41_vm_map_sg() and similars, let those basically deal with card-size
PTEs exclusively, and move the breakdown logic inside
nouveau_vm_map_sg_table() and friendes, that's the only way it can get
pte and pde right anyway and it would generally work with all backends
(well, hopefully)

Now, to do that, I need a better understanding of the various things
in there since I'm not familiar with nouveau at all. What I think I've
figured out is with a few questions, it would be awesome if you could
answer them so I can have a shot at fixing it all :-)

 - There is spg_shift and lpg_shift in the backend vmm. My understanding
is those correspond to the supported small and large page shift respectively
in the card's MMU, correct ? On nv41 they are both 12.

 - vma-node-type indicates the desired page shift for a given vma
object we are trying to map. It may or may not match spg_shift. If it
doesn't, the 'big' flag gets set in the various vm/base.c functions,
which makes them use a different page table via:

struct nouveau_gpuobj *pgt = vm-pgt[pde].obj[big];

 Question: Can that ever happen on nv41 ? Or rather, can node-type
ever be set to something that is neither vmm-spg_shift nor vmm-lpg_shift ?

 - vma-node-offset is the location in bytes in the card memory space
of the nouveau_vma object, right ?

 - In nouveau_vm_map_sg_table(), we take a delta argument. I assume that's
an indication of where within the vma we want to map the sg list, ie page
1 of the sg list gets mapped to page 1 of the VMA, correct ?

Thanks !

Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Fixing nouveau for 4k PAGE_SIZE

[Benjamin Herrenschmidt]

On Sun, 2013-08-11 at 10:41 +1000, Benjamin Herrenschmidt wrote:
 Now, to do that, I need a better understanding of the various things
 in there since I'm not familiar with nouveau at all. What I think I've
 figured out is with a few questions, it would be awesome if you could
 answer them so I can have a shot at fixing it all :-)

Ok, a few more questions :-)

 - in struct nouveau_mm_node, what unit are offset and length ?

They *seem* to be hard wired to be in units of 4k (regardless of either
of system page size) which I assume means card small page size, but
offset is in bytes ? At least according to my parsing of the code in
nouveau_vm_map_at().

The big question then is whether that represents card address space or
system address space... I assume the former right ? So a function like
nouveau_vm_map_at() is solely concerned with mapping a piece of card
address space in the card VM and thus shouldn't be concerned by the
system PAGE_SIZE at all, right ?

IE. The only one we should actually care about here is
nouveau_vm_map_sg_table() or am I missing an important piece of the
puzzle ?

Additionally, nv41.c has only map_sg() callbacks, no map() callbacks,
should I assume that means that nouveau_vm_map() and nouveau_vm_map_at()
will never be called on these ?

 - In vm/base.c this construct appears regulary:

struct nouveau_gpuobj *pgt = vm-pgt[pde].obj[big];

Which makes me believe we have separate page tables for small vs. large
pages (in card mmu) (though I assume big is always 0 on nv40 unless
missed something, I want to make sure I'm not breaking everything
else...).

Thus I assume that each pte in a big page table maps a page size
of 1  vmm-lpg_shift, is that correct ?

vmm-pgt_bits is always the same however, thus I assume that PDEs always
map the same amount of space, but regions for large pages just have
fewer PTEs, which seem to correspond to what the code does here:

u32 pte  = (offset  ((1  vmm-pgt_bits) - 1))  bits;

- My basic idea is to move the card page vs system PAGE breakdown
up into vm/base.c, which seems reasonably simple in the case of
nouveau_vm_map_sg_table() since we only call vmm-map_sg for one PTE at
a time, but things get a bit trickier with nouveau_vm_map_sg which
passes the whole page list down.

Following my idea would mean essentially also making it operate one PTE
at a time, thus potentially reducing the performance of the map
operations.

One option is to special case the PAGE_SIZE==12 case to keep the
existing behaviour and operate in reduced (one PTE per call)
mode in the other case but I dislike special casing like that (bitrots,
one code path gets untested/unfixed, etc...)

Another option would be to make struct nouveau_mem *mem -pages always
be an array of 4k (ie, create a breakdown when constructing that list),
but I have yet to figure out what the impact would be (where that stuff
gets created) and whether this is realistic at all...

 - struct nouveau_mem is called node in various places (in
nouveau_ttm) which is confusing. What is the relationship between
nouveau_mem, nouveau_mm and nouveau_mm_node ? nouveau_mem seems to be
having things such as offset in multiple of PAGE_SIZE, but have a
page_shift member that appears to match the buffer object page_shift,
hence seem to indicate that it is a card page shift...

Would it be possible, maybe, to add comments next to the fields in
those various data structure indicating in which units they are ?

 - Similar confusion arises with things like struct ttm_mem_reg *mem.
For example, in nouveau_ttm.c, I see statements like:

ret = nouveau_vm_get(man-priv, mem-num_pages  12, node-page_shift,
 NV_MEM_ACCESS_RW, node-vma[0]);

Which seems to indicate that mem-num_pages is in multiple of 4k always,
though I would have though that a ttm object was in multiple of
PAGE_SIZE, am I wrong ?

Especially since the same object is later populated using:

mem-start   = node-vma[0].offset  PAGE_SHIFT;

So the start member is in units of PAGE_SHIFT here, not 12.

So I'm still a bit confused :-)

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Fixing nouveau for 4k PAGE_SIZE

[Benjamin Herrenschmidt]

On Sun, 2013-08-11 at 08:17 +0200, Maarten Lankhorst wrote:
  So I'm still a bit confused :-)
 
 The fun has been doubled because TTM expects PAGE units, so some of
 the PAGE_SHIFT's are
 genuine. Some may be a result of PAGE_SHIFT == 12, so honestly I don't
 know the specific ones.

Right, and the other way around too :-)

I think I found at least two cases where 12 was used where it should
have been PAGE_SHIFT (basically ttm_mem_reg-num_pages). This
is only the tip of the iceberg, so this isn't a formal patch submission,
but I would appreciate your thought as to whether the below is correct
(and thus I'm on the right track) :

--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -31,7 +31,7 @@ nv04_sgdma_bind(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
 {
struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
struct nouveau_mem *node = mem-mm_node;
-   u64 size = mem-num_pages  12;
+   u64 size = mem-num_pages  PAGE_SHIFT;
 
if (ttm-sg) {
node-sg = ttm-sg;
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nou
index 19e3757..f0629de 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -252,8 +252,8 @@ nv04_gart_manager_new(struct ttm_mem_type_manager *man,
 
node-page_shift = 12;
 
-   ret = nouveau_vm_get(man-priv, mem-num_pages  12, node-page_shift,
-NV_MEM_ACCESS_RW, node-vma[0]);
+   ret = nouveau_vm_get(man-priv, mem-num_pages  PAGE_SHIFT,
+node-page_shift, NV_MEM_ACCESS_RW, node-vma[0]);
if (ret) {
kfree(node);

Thanks !

Cheers,
Ben.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Fixing nouveau for 4k PAGE_SIZE

[Benjamin Herrenschmidt]

On Sun, 2013-08-11 at 17:06 +1000, Benjamin Herrenschmidt wrote:

 I think I found at least two cases where 12 was used where it should
 have been PAGE_SHIFT (basically ttm_mem_reg-num_pages). This
 is only the tip of the iceberg, so this isn't a formal patch submission,
 but I would appreciate your thought as to whether the below is correct
 (and thus I'm on the right track) :

This patch (which needs cleanups, and probably be broken down for
bisectability) makes it work for me. I've disabled nouveau_dri for now
as this has its own problems related to Ajax recent gallium endian
changes.

Note the horrible duplication of nouveau_vm_map_sg...

I think to fix it cleanly we probably need to slightly change the
-map_sg API to the vmmr. However, I do have a question whose answer
might make things a LOT easier if yes can make things a lot easier:

Can we guarantee that that such an sg object (I assume this is always
a ttm_bo getting placed in the TT memory, correct ?) has an alignment
in *card VM space* that is a multiple of the *system page size* ? Ie,
can we make that happen easily ?

For example, if my system page size is 64k, can we guarantee that it
will always be mapped in the card at a virtual address that is 64k
aligned ?

If that is the case, then we *know* that a given page in the page list
passed to nouveau_vm_map_sg() will never cross a pde boundary (will
always be fully contained in the bottom level of the page table). That
allows to simplify the code a bit, and maybe to write a unified function
that can still pass down page lists to the vmmr

On the other hand, if we have to handle misalignment, then we may as
well stick to 1 PTE at a time like my patch does to avoid horrible
complications.

Cheers,
Ben.

diff --git a/drivers/gpu/drm/nouveau/core/engine/fifo/nv40.c 
b/drivers/gpu/drm/nouveau/core/engine/fifo/nv40.c
index 5c7433d..c314a5f 100644
--- a/drivers/gpu/drm/nouveau/core/engine/fifo/nv40.c
+++ b/drivers/gpu/drm/nouveau/core/engine/fifo/nv40.c
@@ -190,8 +190,8 @@ nv40_fifo_chan_ctor(struct nouveau_object *parent,
if (size  sizeof(*args))
return -EINVAL;
 
-   ret = nouveau_fifo_channel_create(parent, engine, oclass, 0, 0xc0,
- 0x1000, args-pushbuf,
+   ret = nouveau_fifo_channel_create(parent, engine, oclass, 0, 0x80,
+ 0x1, args-pushbuf,
  (1ULL  NVDEV_ENGINE_DMAOBJ) |
  (1ULL  NVDEV_ENGINE_SW) |
  (1ULL  NVDEV_ENGINE_GR) |
diff --git a/drivers/gpu/drm/nouveau/core/subdev/vm/base.c 
b/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
index ef3133e..5833851 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
@@ -84,10 +84,11 @@ nouveau_vm_map_sg_table(struct nouveau_vma *vma, u64 delta, 
u64 length,
 {
struct nouveau_vm *vm = vma-vm;
struct nouveau_vmmgr *vmm = vm-vmm;
-   int big = vma-node-type != vmm-spg_shift;
+   u32 shift = vma-node-type;
+   int big = shift != vmm-spg_shift;
u32 offset = vma-node-offset + (delta  12);
-   u32 bits = vma-node-type - 12;
-   u32 num  = length  vma-node-type;
+   u32 bits = shift - 12;
+   u32 num  = length  shift;
u32 pde  = (offset  vmm-pgt_bits) - vm-fpde;
u32 pte  = (offset  ((1  vmm-pgt_bits) - 1))  bits;
u32 max  = 1  (vmm-pgt_bits - bits);
@@ -98,7 +99,7 @@ nouveau_vm_map_sg_table(struct nouveau_vma *vma, u64 delta, 
u64 length,
 
for_each_sg(mem-sg-sgl, sg, mem-sg-nents, i) {
struct nouveau_gpuobj *pgt = vm-pgt[pde].obj[big];
-   sglen = sg_dma_len(sg)  PAGE_SHIFT;
+   sglen = sg_dma_len(sg)  shift;
 
end = pte + sglen;
if (unlikely(end = max))
@@ -106,7 +107,7 @@ nouveau_vm_map_sg_table(struct nouveau_vma *vma, u64 delta, 
u64 length,
len = end - pte;
 
for (m = 0; m  len; m++) {
-   dma_addr_t addr = sg_dma_address(sg) + (m  
PAGE_SHIFT);
+   dma_addr_t addr = sg_dma_address(sg) + (m  shift);
 
vmm-map_sg(vma, pgt, mem, pte, 1, addr);
num--;
@@ -121,7 +122,7 @@ nouveau_vm_map_sg_table(struct nouveau_vma *vma, u64 delta, 
u64 length,
}
if (m  sglen) {
for (; m  sglen; m++) {
-   dma_addr_t addr = sg_dma_address(sg) + (m  
PAGE_SHIFT);
+   dma_addr_t addr = sg_dma_address(sg) + (m  
shift);
 
vmm-map_sg(vma, pgt, mem, pte, 1, addr);
num--;
@@ -136,6 +137,7 @@ finish:
vmm-flush(vm);
 }
 
+#if PAGE_SHIFT == 12
 void
 nouveau_vm_map_sg(struct nouveau_vma *vma, u64 delta, u64 length

Re: Fixing nouveau for 4k PAGE_SIZE

[Benjamin Herrenschmidt]

On Sun, 2013-08-11 at 11:02 +0200, Maarten Lankhorst wrote:

  diff --git a/drivers/gpu/drm/nouveau/core/engine/fifo/nv40.c 
  b/drivers/gpu/drm/nouveau/core/engine/fifo/nv40.c
  index 5c7433d..c314a5f 100644
  --- a/drivers/gpu/drm/nouveau/core/engine/fifo/nv40.c
  +++ b/drivers/gpu/drm/nouveau/core/engine/fifo/nv40.c
  @@ -190,8 +190,8 @@ nv40_fifo_chan_ctor(struct nouveau_object *parent,
  if (size  sizeof(*args))
  return -EINVAL;
   
  -   ret = nouveau_fifo_channel_create(parent, engine, oclass, 0, 0xc0,
  - 0x1000, args-pushbuf,
  +   ret = nouveau_fifo_channel_create(parent, engine, oclass, 0, 0x80,
  + 0x1, args-pushbuf,
(1ULL  NVDEV_ENGINE_DMAOBJ) |
(1ULL  NVDEV_ENGINE_SW) |
(1ULL  NVDEV_ENGINE_GR) |
 Why the size change?

This reverts the value to older ones, however that patch might not be
needed anymore (I was carrying it from Dave but if we don't map the
registers into userspace we shouldn't need to force align them).

  diff --git a/drivers/gpu/drm/nouveau/core/subdev/vm/base.c 
  b/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
  index ef3133e..5833851 100644
  --- a/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
  +++ b/drivers/gpu/drm/nouveau/core/subdev/vm/base.c
  @@ -84,10 +84,11 @@ nouveau_vm_map_sg_table(struct nouveau_vma *vma, u64 
  delta, u64 length,
   {
  struct nouveau_vm *vm = vma-vm;
  struct nouveau_vmmgr *vmm = vm-vmm;
  -   int big = vma-node-type != vmm-spg_shift;
  +   u32 shift = vma-node-type;
  +   int big = shift != vmm-spg_shift;
  u32 offset = vma-node-offset + (delta  12);
  -   u32 bits = vma-node-type - 12;
  -   u32 num  = length  vma-node-type;
  +   u32 bits = shift - 12;
  +   u32 num  = length  shift;
  u32 pde  = (offset  vmm-pgt_bits) - vm-fpde;
  u32 pte  = (offset  ((1  vmm-pgt_bits) - 1))  bits;
  u32 max  = 1  (vmm-pgt_bits - bits);
  @@ -98,7 +99,7 @@ nouveau_vm_map_sg_table(struct nouveau_vma *vma, u64 
  delta, u64 length,
   
  for_each_sg(mem-sg-sgl, sg, mem-sg-nents, i) {
  struct nouveau_gpuobj *pgt = vm-pgt[pde].obj[big];
  -   sglen = sg_dma_len(sg)  PAGE_SHIFT;
  +   sglen = sg_dma_len(sg)  shift;
   
  end = pte + sglen;
  if (unlikely(end = max))
 Please add a WARN_ON(big); in map_sg and map_sg_table if you do this.

So that's debatable :-)

The above code is map_sg. Arguably my patch *fixes* using it with card
large pages :-)

IE, Look at the usual case (PAGE_SHIFT=12). Today, the above means
sglen will be in units of small pages. But everything else in that loop
operates in units of whatever is represented by the pte, which can
either be 4k or larger. 

So adding sglen to pte was never right for shift != 12 before
(regardless of PAGE_SHIFT).

With my patch, it's more correct, so as such, adding a WARN_ON here
wouldn't be if I do this :-)

Now, the big case still cannot really work here with PAGE_SHIFT=12,
because that would require the sg segments to be multiple of
shift (the card large page) and that is not going to be the case.

So funnily enough, we *could* use card large pages of 64k (big) if ...
we had PAGE_SHIFT=16 ... which we do on ppc64 :-) But not anywhere else.

But yes, the point remains, in the general case, that function cannot
work for the big case, so I wonder if we should catch it with a
WARN_ON and maybe simplify the code a bunch while at it...

  @@ -106,7 +107,7 @@ nouveau_vm_map_sg_table(struct nouveau_vma *vma, u64 
  delta, u64 length,
  len = end - pte;
   
  for (m = 0; m  len; m++) {
  -   dma_addr_t addr = sg_dma_address(sg) + (m  
  PAGE_SHIFT);
  +   dma_addr_t addr = sg_dma_address(sg) + (m  shift);
   
  vmm-map_sg(vma, pgt, mem, pte, 1, addr);
  num--;
  @@ -121,7 +122,7 @@ nouveau_vm_map_sg_table(struct nouveau_vma *vma, u64 
  delta, u64 length,
  }
  if (m  sglen) {
  for (; m  sglen; m++) {
  -   dma_addr_t addr = sg_dma_address(sg) + (m  
  PAGE_SHIFT);
  +   dma_addr_t addr = sg_dma_address(sg) + (m  
  shift);
   
  vmm-map_sg(vma, pgt, mem, pte, 1, addr);
  num--;
  @@ -136,6 +137,7 @@ finish:
  vmm-flush(vm);
   }
   
  +#if PAGE_SHIFT == 12
   void
   nouveau_vm_map_sg(struct nouveau_vma *vma, u64 delta, u64 length,
struct nouveau_mem *mem)
  @@ -143,10 +145,11 @@ nouveau_vm_map_sg(struct nouveau_vma *vma, u64 delta, 
  u64 length,
  struct nouveau_vm *vm = vma-vm;
  struct nouveau_vmmgr *vmm = vm-vmm;
  dma_addr_t *list = mem-pages;
  -   int big = vma-node-type != vmm-spg_shift;
  +   u32 shift = vma-node-type;
  +   int big = 

Re: [PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Sat, 2017-02-18 at 15:43 +, Emil Velikov wrote:
> 
> Out of curiosity - can you try testing with and w/o the ddr_test_2500
> bug fix(?).
> Would be interesting to see if any of the omitted code [in
> ast_dram_init_2500] has noticeable effect.

Afaik, the original ddr_test_2500 from aspeed wasn't buggy, was it ?

I thought I introduced the bug you found when I turned the values
into bools.

Anyway, I can't test the POST code so... I'll have to wait for Y.C.Chen
to do that.

> > I've changed my refactoring of mmc_test_* to be closer to the
> > original
> > code as I had missed a difference in loop exit condition.
> > 
> 
> The series looks a lot, better. Thank you for addressing my
> suggestions !
> 
> Regards
> Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/9] drm/ast: Factor mmc_test code in POST code

[Benjamin Herrenschmidt]

On Sat, 2017-02-18 at 15:28 +, Emil Velikov wrote:
> Original code does the following data flexing only for the mmc_*test2
> code, while this patch adds it to both test and test2.
> 
>    data = ast_mindwm(...);
>    data = (data | (data >> 16)) & 0x;
> //   ast_moutdwm(...);
>    return data;

Actually that reading of the data isn't much of a problem from
my understanding of the spec, however it's the different exit
condition of the loop that could be.

Cheers,
Ben.

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

Re: [PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Fri, 2017-02-17 at 21:27 +, Emil Velikov wrote:
> > Heh ok. I don't want to change that POST code too much as I'm not
> > equipped to test it, but I'll have a look later today.
> > 
> 
> Not sure why you opted for splitting each suggestion in separate
> email, but it seems to have lead to a [serious] bugfix to go
> unnoticed.
> Namely:

Dunno why either. I think I was distracted doing too many things at
once.

> > > > +static bool ddr_test_2500(struct ast_private *ast)
> > > > +{
> > > > +   ast_moutdwm(ast, 0x1E6E0074, 0x);
> > > > +   ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
> > > > +   if (mmc_test_burst(ast, 0) < 0)
> > > > +   return false;
> > > > +   if (mmc_test_burst(ast, 1) < 0)
> > > > +   return false;
> > > > +   if (mmc_test_burst(ast, 2) < 0)
> > > > +   return false;
> > > > +   if (mmc_test_burst(ast, 3) < 0)
> > > > +   return false;
> > > > +   if (mmc_test_single_2500(ast, 0) < 0)
> > > > +   return false;
> > > > +   return false;
> > > 
> > > Final return should be true... either things got funny with v2 or
> > > the
> > > this patch was never tested ?

As I said, never tested, I don't have the means, I'm waiting for Aspeed
to test it, hopefully monday. I can test the basic function but not
POST. I'll send a respin anyway.

Note that the POST patch is purposefully at the end of the series, it
can wait. The reason is that that code is only useful if the BMC has
no code running on it, not even u-boot, and thus its memory controller
needs to be remotely initialized by the host.

Most servers out there have something running on the BMC and all my
POWER9 systems won't boot without something on the BMC making them do
so :-)

So the POST patch can be merged later once it has had more massaging
and testing.

Cheers,
Ben.
> 
> This here.
> 
> Regards,
> Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Thu, 2017-02-16 at 14:24 +1000, Dave Airlie wrote:
> On 11 January 2017 at 12:57, Y.C. Chen 
> wrote:
> > From: "Y.C. Chen" 
> 
> Please run checkpatch over this patch, it's got a lot of bad
> whitespace issues
> (4 space and 2 space indents, indent the timing tables , remove start
> of line whitespace.).

It's also misdetecting VRAM on my 2500 based POWER9 boxes. I'll take
over the patch, make it work and clean it up. Along with fixing the
P2A business properly.

New patch tomorrow hopefully.

Cheers,
Ben.

> 
> Dave.
> 
> > 
> > Signed-off-by: Y.C. Chen 
> > ---
> >  drivers/gpu/drm/ast/ast_drv.h|   2 +
> >  drivers/gpu/drm/ast/ast_main.c   |  27 ++-
> >  drivers/gpu/drm/ast/ast_mode.c   |  25 +-
> >  drivers/gpu/drm/ast/ast_post.c   | 510
> > ++-
> >  drivers/gpu/drm/ast/ast_tables.h |  57 -
> >  5 files changed, 596 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_drv.h
> > b/drivers/gpu/drm/ast/ast_drv.h
> > index 908011d..7e406ce 100644
> > --- a/drivers/gpu/drm/ast/ast_drv.h
> > +++ b/drivers/gpu/drm/ast/ast_drv.h
> > @@ -64,6 +64,7 @@ enum ast_chip {
> > AST2150,
> > AST2300,
> > AST2400,
> > +   AST2500,
> > AST1180,
> >  };
> > 
> > @@ -80,6 +81,7 @@ enum ast_tx_chip {
> >  #define AST_DRAM_1Gx32   3
> >  #define AST_DRAM_2Gx16   6
> >  #define AST_DRAM_4Gx16   7
> > +#define AST_DRAM_8Gx16   8
> > 
> >  struct ast_fbdev;
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_main.c
> > b/drivers/gpu/drm/ast/ast_main.c
> > index f75c642..40460ce 100644
> > --- a/drivers/gpu/drm/ast/ast_main.c
> > +++ b/drivers/gpu/drm/ast/ast_main.c
> > @@ -73,7 +73,10 @@ static int ast_detect_chip(struct drm_device
> > *dev, bool *need_post)
> > ast->chip = AST1100;
> > DRM_INFO("AST 1180 detected\n");
> > } else {
> > -   if (dev->pdev->revision >= 0x30) {
> > +   if (dev->pdev->revision >= 0x40) {
> > +   ast->chip = AST2500;
> > +   DRM_INFO("AST 2500 detected\n");
> > +   } else if (dev->pdev->revision >= 0x30) {
> > ast->chip = AST2400;
> > DRM_INFO("AST 2400 detected\n");
> > } else if (dev->pdev->revision >= 0x20) {
> > @@ -149,6 +152,8 @@ static int ast_detect_chip(struct drm_device
> > *dev, bool *need_post)
> > ast->support_wide_screen = true;
> > if (ast->chip == AST2400 && data == 0x100)
> > /* ast1400 */
> > ast->support_wide_screen = true;
> > +   if (ast->chip == AST2500 && data == 0x100)
> > /* ast2510 */
> > +   ast->support_wide_screen = true;
> > }
> > break;
> > }
> > @@ -233,7 +238,24 @@ static int ast_get_dram_info(struct drm_device
> > *dev)
> > else
> > ast->dram_bus_width = 32;
> > 
> > -   if (ast->chip == AST2300 || ast->chip == AST2400) {
> > +   if (ast->chip == AST2500) {
> > +   switch (data & 0x03) {
> > +   case 0:
> > +   ast->dram_type = AST_DRAM_1Gx16;
> > +   break;
> > +   default:
> > +   case 1:
> > +   ast->dram_type = AST_DRAM_2Gx16;
> > +   break;
> > +   case 2:
> > +   ast->dram_type = AST_DRAM_4Gx16;
> > +   break;
> > +   case 3:
> > +   ast->dram_type = AST_DRAM_8Gx16;
> > +   break;
> > +   }
> > +   }
> > +   else if (ast->chip == AST2300 || ast->chip == AST2400) {
> > switch (data & 0x03) {
> > case 0:
> > ast->dram_type = AST_DRAM_512Mx16;
> > @@ -456,6 +478,7 @@ int ast_driver_load(struct drm_device *dev,
> > unsigned long flags)
> > ast->chip == AST2200 ||
> > ast->chip == AST2300 ||
> > ast->chip == AST2400 ||
> > +   ast->chip == AST2500 ||
> > ast->chip == AST1180) {
> > dev->mode_config.max_width = 1920;
> > dev->mode_config.max_height = 2048;
> > diff --git a/drivers/gpu/drm/ast/ast_mode.c
> > b/drivers/gpu/drm/ast/ast_mode.c
> > index e26c98f..242ca7f 100644
> > --- a/drivers/gpu/drm/ast/ast_mode.c
> > +++ b/drivers/gpu/drm/ast/ast_mode.c
> > @@ -271,7 +271,10 @@ static void ast_set_crtc_reg(struct drm_crtc
> > *crtc, struct drm_display_mode *mod
> >  {
> > struct ast_private *ast = crtc->dev->dev_private;
> > u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD =
> > 0, jregAE = 0;
> > -   u16 temp;
> > +   u16 temp, precache = 0;
> > +
> > +if ((ast->chip == AST2500) && 

Re: [PATCH] drm/ast: Fixed system hanged if disable P2A

[Benjamin Herrenschmidt]

On Thu, 2017-01-26 at 09:45 +0800, Y.C. Chen wrote:
> From: "Y.C. Chen" 
> 
> The original ast driver will access some BMC configuration through
> P2A bridge
> that can be disabled since AST2300 and after.
> It will cause system hanged if P2A bridge is disabled.
> Here is the update to fix it.

Will this work for us on POWER ? The problem we observed is that if P2A
is disabled, the read from f00x seems to not respond, ie, we get a PCIe
target abort on the MMIO, not 0x_. That trips our HW error
detection mechanism "EEH" which will take out the entire device.

Russell (CC) proposed a patch a while ago that seems to have gone
nowhere that allows the driver to retrieve all the information it
needs from the device-tree on platforms that support it.

That allows us to grab the necessary info from firmware before the
bridge is closed (or via some specific BMC interface we have if we
close the bridge) and pass it to Linux.

Any reason that patch hasn't been merged ? Dave ?

Cheers,
Ben.


> Signed-off-by: Y.C. Chen 
> ---
>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
>  drivers/gpu/drm/ast/ast_main.c | 156 ++-
> --
>  drivers/gpu/drm/ast/ast_post.c |  18 +++--
>  3 files changed, 96 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h
> b/drivers/gpu/drm/ast/ast_drv.h
> index 908011d..7abda94 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -113,6 +113,7 @@ struct ast_private {
>   struct ttm_bo_kmap_obj cache_kmap;
>   int next_cursor;
>   bool support_wide_screen;
> + bool DisableP2A;
>  
>   enum ast_tx_chip tx_chip_type;
>   u8 dp501_maxclk;
> diff --git a/drivers/gpu/drm/ast/ast_main.c
> b/drivers/gpu/drm/ast/ast_main.c
> index f75c642..c374685 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -124,6 +124,11 @@ static int ast_detect_chip(struct drm_device
> *dev, bool *need_post)
>   } else
>   *need_post = false;
>  
> + /* Check P2A Access */
> + ast->DisableP2A = true;
> + data = ast_read32(ast, 0xf004);
> + if (data != 0x) ast->DisableP2A = false;
> +
>   /* Check if we support wide screen */
>   switch (ast->chip) {
>   case AST1180:
> @@ -140,15 +145,17 @@ static int ast_detect_chip(struct drm_device
> *dev, bool *need_post)
>   ast->support_wide_screen = true;
>   else {
>   ast->support_wide_screen = false;
> - /* Read SCU7c (silicon revision register) */
> - ast_write32(ast, 0xf004, 0x1e6e);
> - ast_write32(ast, 0xf000, 0x1);
> - data = ast_read32(ast, 0x1207c);
> - data &= 0x300;
> - if (ast->chip == AST2300 && data == 0x0) /*
> ast1300 */
> - ast->support_wide_screen = true;
> - if (ast->chip == AST2400 && data == 0x100)
> /* ast1400 */
> - ast->support_wide_screen = true;
> + if (ast->DisableP2A == false) {
> + /* Read SCU7c (silicon revision
> register) */
> + ast_write32(ast, 0xf004,
> 0x1e6e);
> + ast_write32(ast, 0xf000, 0x1);
> + data = ast_read32(ast, 0x1207c);
> + data &= 0x300;
> + if (ast->chip == AST2300 && data ==
> 0x0) /* ast1300 */
> + ast->support_wide_screen =
> true;
> + if (ast->chip == AST2400 && data ==
> 0x100) /* ast1400 */
> + ast->support_wide_screen =
> true;
> + }
>   }
>   break;
>   }
> @@ -216,80 +223,81 @@ static int ast_get_dram_info(struct drm_device
> *dev)
>   uint32_t data, data2;
>   uint32_t denum, num, div, ref_pll;
>  
> - ast_write32(ast, 0xf004, 0x1e6e);
> - ast_write32(ast, 0xf000, 0x1);
> -
> -
> - ast_write32(ast, 0x1, 0xfc600309);
> -
> - do {
> - if (pci_channel_offline(dev->pdev))
> - return -EIO;
> - } while (ast_read32(ast, 0x1) != 0x01);
> - data = ast_read32(ast, 0x10004);
> -
> - if (data & 0x40)
> + if (ast->DisableP2A)
> + {
>   ast->dram_bus_width = 16;
> + ast->dram_type = AST_DRAM_1Gx16;
> + ast->mclk = 396;
> + }
>   else
> - ast->dram_bus_width = 32;
> + {
> + ast_write32(ast, 0xf004, 0x1e6e);
> + ast_write32(ast, 0xf000, 0x1);
> + data = ast_read32(ast, 0x10004);
> +
> + if (data & 0x40)
> + ast->dram_bus_width = 16;
> + else
> + 

Re: [PATCH] drm/ast: Fixed system hanged if disable P2A

[Benjamin Herrenschmidt]

On Thu, 2017-02-16 at 06:34 +, YC Chen wrote:
> Hi Ben,
> This solution had been proved on x86 platform but I'm not sure if it
> works or not on POWER system .

It will only work on x86 platforms that haven't configured their
PCI Express in "hardened" mode where PCIe errors cause machine checks
or worse.

That said there isn't much I can do about it for now. So for POWER
platforms (and other platforms using device-tree), I have a patch
that will get the data from there.

>  If we added a definition in SOC scratch register to info drm driver
> the P2A is disabled by BMC fw, then drm driver will no need to check
> it by reading 0xf00x. Is it acceptable?

It would be better yes. Please send me a patch as I've reworked yours
already (see what i'll post tomorrow). Additionally, for future
silicon, please make all those info necessary to detect the VRAM
and the PLL available via the VGA registers so the backdoor isn't
needed.

For a full POST we can still require it of course but for the normal
case we should be able to get all the info needed via the normal VGA
register space.

> Here are the flow:
> If disable P2A in SOC scratch then ast->DisableP2A = true;
> Else if the value of 0xf00x is not 0x_ then ast->DisableP2A =
> false;

Cheers,
Ben.

> Regards,
> 
> Y.C. Chen
> 
> -Original Message-
> From: Benjamin Herrenschmidt [mailto:b...@au1.ibm.com] 
> Sent: Thursday, February 16, 2017 11:58 AM
> To: YC Chen <yc_c...@aspeedtech.com>; dri-devel@lists.freedesktop.org
> Cc: airl...@redhat.com; e...@suse.com; Russell Currey <rus...@au1.ibm
> .com>
> Subject: Re: [PATCH] drm/ast: Fixed system hanged if disable P2A
> 
> On Thu, 2017-01-26 at 09:45 +0800, Y.C. Chen wrote:
> > From: "Y.C. Chen" <yc_c...@aspeedtech.com>
> > 
> > The original ast driver will access some BMC configuration through
> > P2A 
> > bridge that can be disabled since AST2300 and after.
> > It will cause system hanged if P2A bridge is disabled.
> > Here is the update to fix it.
> 
> Will this work for us on POWER ? The problem we observed is that if
> P2A is disabled, the read from f00x seems to not respond, ie, we get
> a PCIe target abort on the MMIO, not 0x_. That trips our HW
> error detection mechanism "EEH" which will take out the entire
> device.
> 
> Russell (CC) proposed a patch a while ago that seems to have gone
> nowhere that allows the driver to retrieve all the information it
> needs from the device-tree on platforms that support it.
> 
> That allows us to grab the necessary info from firmware before the
> bridge is closed (or via some specific BMC interface we have if we
> close the bridge) and pass it to Linux.
> 
> Any reason that patch hasn't been merged ? Dave ?
> 
> Cheers,
> Ben.
> 
> 
> > Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
> > ---
> >  drivers/gpu/drm/ast/ast_drv.h  |   1 +
> >  drivers/gpu/drm/ast/ast_main.c | 156 ++---
> > --
> > --
> >  drivers/gpu/drm/ast/ast_post.c |  18 +++--
> >  3 files changed, 96 insertions(+), 79 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_drv.h 
> > b/drivers/gpu/drm/ast/ast_drv.h index 908011d..7abda94 100644
> > --- a/drivers/gpu/drm/ast/ast_drv.h
> > +++ b/drivers/gpu/drm/ast/ast_drv.h
> > @@ -113,6 +113,7 @@ struct ast_private {
> >     struct ttm_bo_kmap_obj cache_kmap;
> >     int next_cursor;
> >     bool support_wide_screen;
> > +   bool DisableP2A;
> >  
> >     enum ast_tx_chip tx_chip_type;
> >     u8 dp501_maxclk;
> > diff --git a/drivers/gpu/drm/ast/ast_main.c 
> > b/drivers/gpu/drm/ast/ast_main.c index f75c642..c374685 100644
> > --- a/drivers/gpu/drm/ast/ast_main.c
> > +++ b/drivers/gpu/drm/ast/ast_main.c
> > @@ -124,6 +124,11 @@ static int ast_detect_chip(struct drm_device 
> > *dev, bool *need_post)
> >     } else
> >     *need_post = false;
> >  
> > +   /* Check P2A Access */
> > +   ast->DisableP2A = true;
> > +   data = ast_read32(ast, 0xf004);
> > +   if (data != 0x) ast->DisableP2A = false;
> > +
> >     /* Check if we support wide screen */
> >     switch (ast->chip) {
> >     case AST1180:
> > @@ -140,15 +145,17 @@ static int ast_detect_chip(struct drm_device 
> > *dev, bool *need_post)
> >     ast->support_wide_screen = true;
> >     else {
> >     ast->support_wide_screen = false;
> > -   /* Read SCU7c (silicon revision register)
> > */
> > -   ast_write32(ast, 0

Re: [PATCH] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Wed, 2017-01-11 at 10:57 +0800, Y.C. Chen wrote:
> From: "Y.C. Chen" 
> 
> Signed-off-by: Y.C. Chen 
> ---
>  drivers/gpu/drm/ast/ast_drv.h|   2 +
>  drivers/gpu/drm/ast/ast_main.c   |  27 ++-
>  drivers/gpu/drm/ast/ast_mode.c   |  25 +-
>  drivers/gpu/drm/ast/ast_post.c   | 510
> ++-
>  drivers/gpu/drm/ast/ast_tables.h |  57 -
>  5 files changed, 596 insertions(+), 25 deletions(-)

Dave, Daniel, is that going upstream ? We need that for POWER9
machines, it needs to hit the distros ASAP.

We also need the one Russell Currey sent a while ago

"[PATCH] drivers/gpu/drm/ast: Support reading configuration values from
device tree"

Cheers,
Ben.

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

Re: [PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Fri, 2017-02-17 at 21:27 +, Emil Velikov wrote:
> Hi Ben,

 .../...

> Not sure why you opted for splitting each suggestion in separate
> email, but it seems to have lead to a [serious] bugfix to go
> unnoticed.

Many thanks for your reviews. I've put a tentative new series there

https://github.com/ozbenh/linux-ast

I'm waiting for Aspeed to test on Monday on their HW (they can test
the POST code) and I'll be testing again on POWER8 and POWER9 this
week-end. If all goes well I'll send the new series to the list then.

I've changed my refactoring of mmc_test_* to be closer to the original
code as I had missed a difference in loop exit condition.

Cheers,
Ben.

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

Re: [PATCH 1/9] drm/ast: Handle configuration without P2A bridge

[Benjamin Herrenschmidt]

On Fri, 2017-02-17 at 16:32 +1100, Benjamin Herrenschmidt wrote:

Make this

From: Russell Currey <rus...@russell.cc>

Git ate it when I amended (citool bug) and I forgot to fix it up.

> The ast driver configures a window to enable access into BMC
> memory space in order to read some configuration registers.
> 
> If this window is disabled, which it can be from the BMC side,
> the ast driver can't function.
> 
> Closing this window is a necessity for security if a machine's
> host side and BMC side are controlled by different parties;
> i.e. a cloud provider offering machines "bare metal".
> 
> A recent patch went in to try to check if that window is open
> but it does so by trying to access the registers in question
> and testing if the result is 0x.
> 
> This method will trigger a PCIe error when the window is closed
> which on some systems will be fatal (it will trigger an EEH
> for example on POWER which will take out the device).
> 
> This patch improves this in two ways:
> 
>  - First, if the firmware has put properties in the device-tree
> containing the relevant configuration information, we use these.
> 
>  - Otherwise, a bit in one of the SCU scratch registers (which
> are readable via the VGA register space and writeable by the BMC)
> will indicate if the BMC has closed the window. This bit has been
> defined by Y.C Chen from Aspeed.
> 
> If the window is closed and the configuration isn't available from
> the device-tree, some sane defaults are used. Those defaults are
> hopefully sufficient for standard video modes used on a server.
> 
> Signed-off-by: Russell Currey <rus...@russell.cc>
> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> --
> 
> v2. [BenH]
> - Reworked on top of Aspeed P2A patch
> - Cleanup overall detection via a "config_mode" and log the
>   selected mode for diagnostics purposes
> - Add a property for the SCU straps
> 
> v3. [BenH]
> - Moved the config mode detection to a separate functionn
> - Add reading of SCU 0x40 D[12] to detect the window is
>   closed as to not trigger a bus error by just "trying".
>   (change provided by Y.C. Chen)
> ---
>  drivers/gpu/drm/ast/ast_drv.h  |   6 +-
>  drivers/gpu/drm/ast/ast_main.c | 223 +
> 
>  drivers/gpu/drm/ast/ast_post.c |   7 +-
>  3 files changed, 141 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h
> b/drivers/gpu/drm/ast/ast_drv.h
> index 7abda94..3bedcf7 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -113,7 +113,11 @@ struct ast_private {
>   struct ttm_bo_kmap_obj cache_kmap;
>   int next_cursor;
>   bool support_wide_screen;
> - bool DisableP2A;
> + enum {
> + ast_use_p2a,
> + ast_use_dt,
> + ast_use_defaults
> + } config_mode;
>  
>   enum ast_tx_chip tx_chip_type;
>   u8 dp501_maxclk;
> diff --git a/drivers/gpu/drm/ast/ast_main.c
> b/drivers/gpu/drm/ast/ast_main.c
> index 533e762..823c68f 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -62,13 +62,58 @@ uint8_t ast_get_index_reg_mask(struct ast_private
> *ast,
>   return ret;
>  }
>  
> +static void ast_detect_config_mode(struct drm_device *dev, u32
> *scu_rev)
> +{
> + struct device_node *np = dev->pdev->dev.of_node;
> + struct ast_private *ast = dev->dev_private;
> + uint32_t data, jreg;
> +
> + /* Check if we have device-tree properties */
> + if (np && !of_property_read_u32(np, "ast,scu-revision-id",
> scu_rev)) {
> + /* We do, disable P2A access */
> + ast->config_mode = ast_use_dt;
> + DRM_INFO("Using device-tree for configuration\n");
> + return;
> + }
> +
> + /*
> +  * The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
> +  * is disabled
> +  */
> + jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1,
> 0xff);
> + if (!(jreg & 0x10)) {
> + /* Double check it's actually working */
> + data = ast_read32(ast, 0xf004);
> + if (data != 0x) {
> + /* P2A works, grab silicon revision */
> + ast->config_mode = ast_use_p2a;
> +
> + DRM_INFO("Using P2A bridge for
> configuration\n");
> +
> + /* Read SCU7c (silicon revision register) */
> + ast_write32(ast, 0xf004, 0x1e6e);
> + ast

[PATCH 7/9] drm/ast: Rename ast_init_dram_2300 to ast_post_chip_2300

[Benjamin Herrenschmidt]

The function does more than initializing the DRAM and in turns
calls other functions to do the actual init. This will keeping
things more consistent with the upcoming AST2500 POST code.

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_post.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 33ea1ea..a36a544 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -31,7 +31,7 @@
 
 #include "ast_dram_tables.h"
 
-static void ast_init_dram_2300(struct drm_device *dev);
+static void ast_post_chip_2300(struct drm_device *dev);
 
 void ast_enable_vga(struct drm_device *dev)
 {
@@ -381,7 +381,7 @@ void ast_post_gpu(struct drm_device *dev)
 
if (ast->config_mode == ast_use_p2a) {
if (ast->chip == AST2300 || ast->chip == AST2400)
-   ast_init_dram_2300(dev);
+   ast_post_chip_2300(dev);
else
ast_init_dram_reg(dev);
 
@@ -1577,7 +1577,7 @@ static void ddr2_init(struct ast_private *ast, struct 
ast2300_dram_param *param)
 
 }
 
-static void ast_init_dram_2300(struct drm_device *dev)
+static void ast_post_chip_2300(struct drm_device *dev)
 {
struct ast_private *ast = dev->dev_private;
struct ast2300_dram_param param;
-- 
2.9.3

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

[PATCH 2/9] drm/ast: const'ify mode setting tables

[Benjamin Herrenschmidt]

And fix some comment alignment & space/tabs while at it

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_drv.h|   4 +-
 drivers/gpu/drm/ast/ast_mode.c   |   8 +--
 drivers/gpu/drm/ast/ast_tables.h | 106 +++
 3 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 3bedcf7..3fd9d6e 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -304,8 +304,8 @@ struct ast_vbios_dclk_info {
 };
 
 struct ast_vbios_mode_info {
-   struct ast_vbios_stdtable *std_table;
-   struct ast_vbios_enhtable *enh_table;
+   const struct ast_vbios_stdtable *std_table;
+   const struct ast_vbios_enhtable *enh_table;
 };
 
 extern int ast_mode_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index e26c98f..1ff596e 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -80,9 +80,9 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, 
struct drm_display_mo
 {
struct ast_private *ast = crtc->dev->dev_private;
u32 refresh_rate_index = 0, mode_id, color_index, refresh_rate;
+   const struct ast_vbios_enhtable *best = NULL;
u32 hborder, vborder;
bool check_sync;
-   struct ast_vbios_enhtable *best = NULL;
 
switch (crtc->primary->fb->bits_per_pixel) {
case 8:
@@ -146,7 +146,7 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, 
struct drm_display_mo
refresh_rate = drm_mode_vrefresh(mode);
check_sync = vbios_mode->enh_table->flags & WideScreenMode;
do {
-   struct ast_vbios_enhtable *loop = vbios_mode->enh_table;
+   const struct ast_vbios_enhtable *loop = vbios_mode->enh_table;
 
while (loop->refresh_rate != 0xff) {
if ((check_sync) &&
@@ -225,7 +225,7 @@ static void ast_set_std_reg(struct drm_crtc *crtc, struct 
drm_display_mode *mode
struct ast_vbios_mode_info *vbios_mode)
 {
struct ast_private *ast = crtc->dev->dev_private;
-   struct ast_vbios_stdtable *stdtable;
+   const struct ast_vbios_stdtable *stdtable;
u32 i;
u8 jreg;
 
@@ -381,7 +381,7 @@ static void ast_set_dclk_reg(struct drm_device *dev, struct 
drm_display_mode *mo
 struct ast_vbios_mode_info *vbios_mode)
 {
struct ast_private *ast = dev->dev_private;
-   struct ast_vbios_dclk_info *clk_info;
+   const struct ast_vbios_dclk_info *clk_info;
 
clk_info = _table[vbios_mode->enh_table->dclk_index];
 
diff --git a/drivers/gpu/drm/ast/ast_tables.h b/drivers/gpu/drm/ast/ast_tables.h
index 3608d5a..a4ddf90 100644
--- a/drivers/gpu/drm/ast/ast_tables.h
+++ b/drivers/gpu/drm/ast/ast_tables.h
@@ -78,37 +78,37 @@
 #define VCLK97_75  0x19
 #define VCLK118_25 0x1A
 
-static struct ast_vbios_dclk_info dclk_table[] = {
-   {0x2C, 0xE7, 0x03}, /* 00: 
VCLK25_175   */
-   {0x95, 0x62, 0x03}, /* 01: 
VCLK28_322   */
-   {0x67, 0x63, 0x01}, /* 02: VCLK31_5 
*/
-   {0x76, 0x63, 0x01}, /* 03: VCLK36   
*/
-   {0xEE, 0x67, 0x01}, /* 04: VCLK40   
*/
-   {0x82, 0x62, 0x01}, /* 05: VCLK49_5 
*/
-   {0xC6, 0x64, 0x01}, /* 06: VCLK50   
*/
-   {0x94, 0x62, 0x01}, /* 07: 
VCLK56_25*/
-   {0x80, 0x64, 0x00}, /* 08: VCLK65   
*/
-   {0x7B, 0x63, 0x00}, /* 09: VCLK75   
*/
-   {0x67, 0x62, 0x00}, /* 0A: 
VCLK78_75*/
-   {0x7C, 0x62, 0x00}, /* 0B: VCLK94_5 
*/
-   {0x8E, 0x62, 0x00}, /* 0C: VCLK108  
*/
-   {0x85, 0x24, 0x00}, /* 0D: VCLK135  
*/
-   {0x67, 0x22, 0x00}, /* 0E: 
VCLK157_5*/
-   {0x6A, 0x22, 0x00}, /* 0F: VCLK162  
*/
-   {0x4d, 0x4c, 0x80}, /* 10: VCLK154  
*/
-   {0xa7, 0x78, 0x80}, /* 11: VCLK83.5 
*/
-   {0x28, 0x49, 0x80}, /* 12: 
VCLK106.5*/
-   {0x37, 0x49, 0x80}, /* 13: 
VCLK146.25   */
-   {0x1f,

[PATCH 1/9] drm/ast: Handle configuration without P2A bridge

[Benjamin Herrenschmidt]

The ast driver configures a window to enable access into BMC
memory space in order to read some configuration registers.

If this window is disabled, which it can be from the BMC side,
the ast driver can't function.

Closing this window is a necessity for security if a machine's
host side and BMC side are controlled by different parties;
i.e. a cloud provider offering machines "bare metal".

A recent patch went in to try to check if that window is open
but it does so by trying to access the registers in question
and testing if the result is 0x.

This method will trigger a PCIe error when the window is closed
which on some systems will be fatal (it will trigger an EEH
for example on POWER which will take out the device).

This patch improves this in two ways:

 - First, if the firmware has put properties in the device-tree
containing the relevant configuration information, we use these.

 - Otherwise, a bit in one of the SCU scratch registers (which
are readable via the VGA register space and writeable by the BMC)
will indicate if the BMC has closed the window. This bit has been
defined by Y.C Chen from Aspeed.

If the window is closed and the configuration isn't available from
the device-tree, some sane defaults are used. Those defaults are
hopefully sufficient for standard video modes used on a server.

Signed-off-by: Russell Currey <rus...@russell.cc>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
--

v2. [BenH]
- Reworked on top of Aspeed P2A patch
- Cleanup overall detection via a "config_mode" and log the
  selected mode for diagnostics purposes
- Add a property for the SCU straps

v3. [BenH]
- Moved the config mode detection to a separate functionn
- Add reading of SCU 0x40 D[12] to detect the window is
  closed as to not trigger a bus error by just "trying".
  (change provided by Y.C. Chen)
---
 drivers/gpu/drm/ast/ast_drv.h  |   6 +-
 drivers/gpu/drm/ast/ast_main.c | 223 +
 drivers/gpu/drm/ast/ast_post.c |   7 +-
 3 files changed, 141 insertions(+), 95 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 7abda94..3bedcf7 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -113,7 +113,11 @@ struct ast_private {
struct ttm_bo_kmap_obj cache_kmap;
int next_cursor;
bool support_wide_screen;
-   bool DisableP2A;
+   enum {
+   ast_use_p2a,
+   ast_use_dt,
+   ast_use_defaults
+   } config_mode;
 
enum ast_tx_chip tx_chip_type;
u8 dp501_maxclk;
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 533e762..823c68f 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -62,13 +62,58 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
return ret;
 }
 
+static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
+{
+   struct device_node *np = dev->pdev->dev.of_node;
+   struct ast_private *ast = dev->dev_private;
+   uint32_t data, jreg;
+
+   /* Check if we have device-tree properties */
+   if (np && !of_property_read_u32(np, "ast,scu-revision-id", scu_rev)) {
+   /* We do, disable P2A access */
+   ast->config_mode = ast_use_dt;
+   DRM_INFO("Using device-tree for configuration\n");
+   return;
+   }
+
+   /*
+* The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
+* is disabled
+*/
+   jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
+   if (!(jreg & 0x10)) {
+   /* Double check it's actually working */
+   data = ast_read32(ast, 0xf004);
+   if (data != 0x) {
+   /* P2A works, grab silicon revision */
+   ast->config_mode = ast_use_p2a;
+
+   DRM_INFO("Using P2A bridge for configuration\n");
+
+   /* Read SCU7c (silicon revision register) */
+   ast_write32(ast, 0xf004, 0x1e6e);
+   ast_write32(ast, 0xf000, 0x1);
+   *scu_rev = ast_read32(ast, 0x1207c);
+   return;
+   }
+   }
+
+   DRM_INFO("P2A bridge disabled, using default configuration\n");
+   ast->config_mode = ast_use_defaults;
+   *scu_rev = 0x;
+}
 
 static int ast_detect_chip(struct drm_device *dev, bool *need_post)
 {
struct ast_private *ast = dev->dev_private;
-   uint32_t data, jreg;
+   uint32_t jreg, scu_rev;
+
ast_open_key(ast);
 
+   /* Find out whether P2A works or whether to use device-tree */
+   ast_detect_config_mode(dev, _rev);
+
+   /* Identify chipset */
if (dev->

[PATCH 6/9] drm/ast: Factor mmc_test code in POST code

[Benjamin Herrenschmidt]

There's a lot of duplication for what's essentially N copies of
the same loop, so factor it. The upcoming AST2500 POST code adds
more of this.

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_post.c | 76 --
 1 file changed, 22 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 7197635..33ea1ea 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -445,85 +445,53 @@ static const u32 pattern[8] = {
0x7C61D253
 };
 
-static int mmc_test_burst(struct ast_private *ast, u32 datagen)
+static int mmc_test(struct ast_private *ast, u32 datagen, u8 test_ctl)
 {
u32 data, timeout;
 
ast_moutdwm(ast, 0x1e6e0070, 0x);
-   ast_moutdwm(ast, 0x1e6e0070, 0x00c1 | (datagen << 3));
+   ast_moutdwm(ast, 0x1e6e0070, (datagen << 3) | test_ctl);
timeout = 0;
do {
data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
if (data & 0x2000) {
-   return 0;
+   return -1;
}
if (++timeout > TIMEOUT) {
ast_moutdwm(ast, 0x1e6e0070, 0x);
-   return 0;
+   return -1;
}
} while (!data);
+   data = ast_mindwm(ast, 0x1e6e0078);
+   data = (data | (data >> 16)) & 0x;
ast_moutdwm(ast, 0x1e6e0070, 0x);
-   return 1;
+   return data;
 }
 
-static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
+
+static int mmc_test_burst(struct ast_private *ast, u32 datagen)
 {
-   u32 data, timeout;
+   return mmc_test(ast, datagen, 0xc1);
+}
 
-   ast_moutdwm(ast, 0x1e6e0070, 0x);
-   ast_moutdwm(ast, 0x1e6e0070, 0x0041 | (datagen << 3));
-   timeout = 0;
-   do {
-   data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
-   if (++timeout > TIMEOUT) {
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return -1;
-   }
-   } while (!data);
-   data = ast_mindwm(ast, 0x1e6e0078);
-   data = (data | (data >> 16)) & 0x;
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return data;
+static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
+{
+   return mmc_test(ast, datagen, 0x41);
 }
 
 static int mmc_test_single(struct ast_private *ast, u32 datagen)
 {
-   u32 data, timeout;
-
-   ast_moutdwm(ast, 0x1e6e0070, 0x);
-   ast_moutdwm(ast, 0x1e6e0070, 0x00c5 | (datagen << 3));
-   timeout = 0;
-   do {
-   data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
-   if (data & 0x2000)
-   return 0;
-   if (++timeout > TIMEOUT) {
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return 0;
-   }
-   } while (!data);
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return 1;
+   return mmc_test(ast, datagen, 0xc5);
 }
 
 static int mmc_test_single2(struct ast_private *ast, u32 datagen)
 {
-   u32 data, timeout;
+   return mmc_test(ast, datagen, 0x05);
+}
 
-   ast_moutdwm(ast, 0x1e6e0070, 0x);
-   ast_moutdwm(ast, 0x1e6e0070, 0x0005 | (datagen << 3));
-   timeout = 0;
-   do {
-   data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
-   if (++timeout > TIMEOUT) {
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return -1;
-   }
-   } while (!data);
-   data = ast_mindwm(ast, 0x1e6e0078);
-   data = (data | (data >> 16)) & 0x;
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return data;
+static int mmc_test_single_2500(struct ast_private *ast, u32 datagen)
+{
+   return mmc_test(ast, datagen, 0x85);
 }
 
 static int cbr_test(struct ast_private *ast)
@@ -603,9 +571,9 @@ static u32 cbr_scan2(struct ast_private *ast)
 
 static u32 cbr_test3(struct ast_private *ast)
 {
-   if (!mmc_test_burst(ast, 0))
+   if (mmc_test_burst(ast, 0) < 0)
return 0;
-   if (!mmc_test_single(ast, 0))
+   if (mmc_test_single(ast, 0) < 0)
return 0;
return 1;
 }
-- 
2.9.3

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

[PATCH 5/9] drm/ast: Base support for AST2500

[Benjamin Herrenschmidt]

From: "Y.C. Chen" <yc_c...@aspeedtech.com>

Add detection and mode setting updates for AST2500 generation chip,
code originally from Aspeed and slightly reworked for coding style
mostly by Ben. This doesn't contain the BMC DRAM POST code which
is in a separate patch.

Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
---
 drivers/gpu/drm/ast/ast_drv.h|  2 ++
 drivers/gpu/drm/ast/ast_main.c   | 27 +--
 drivers/gpu/drm/ast/ast_mode.c   | 30 -
 drivers/gpu/drm/ast/ast_tables.h | 58 +---
 4 files changed, 99 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 3fd9d6e..d1c1d53 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -64,6 +64,7 @@ enum ast_chip {
AST2150,
AST2300,
AST2400,
+   AST2500,
AST1180,
 };
 
@@ -80,6 +81,7 @@ enum ast_tx_chip {
 #define AST_DRAM_1Gx32   3
 #define AST_DRAM_2Gx16   6
 #define AST_DRAM_4Gx16   7
+#define AST_DRAM_8Gx16   8
 
 struct ast_fbdev;
 
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index eb6ba3e..f9128b9 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -116,7 +116,10 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
ast->chip = AST1100;
DRM_INFO("AST 1180 detected\n");
} else {
-   if (dev->pdev->revision >= 0x30) {
+   if (dev->pdev->revision >= 0x40) {
+   ast->chip = AST2500;
+   DRM_INFO("AST 2500 detected\n");
+   } else if (dev->pdev->revision >= 0x30) {
ast->chip = AST2400;
DRM_INFO("AST 2400 detected\n");
} else if (dev->pdev->revision >= 0x20) {
@@ -184,6 +187,9 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
if (ast->chip == AST2400 &&
(scu_rev & 0x300) == 0x100) /* ast1400 */
ast->support_wide_screen = true;
+   if (ast->chip == AST2500 &&
+   scu_rev == 0x100)   /* ast2510 */
+   ast->support_wide_screen = true;
}
break;
}
@@ -287,7 +293,23 @@ static int ast_get_dram_info(struct drm_device *dev)
else
ast->dram_bus_width = 32;
 
-   if (ast->chip == AST2300 || ast->chip == AST2400) {
+   if (ast->chip == AST2500) {
+   switch (mcr_cfg & 0x03) {
+   case 0:
+   ast->dram_type = AST_DRAM_1Gx16;
+   break;
+   default:
+   case 1:
+   ast->dram_type = AST_DRAM_2Gx16;
+   break;
+   case 2:
+   ast->dram_type = AST_DRAM_4Gx16;
+   break;
+   case 3:
+   ast->dram_type = AST_DRAM_8Gx16;
+   break;
+   }
+   } else if (ast->chip == AST2300 || ast->chip == AST2400) {
switch (mcr_cfg & 0x03) {
case 0:
ast->dram_type = AST_DRAM_512Mx16;
@@ -510,6 +532,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
ast->chip == AST2200 ||
ast->chip == AST2300 ||
ast->chip == AST2400 ||
+   ast->chip == AST2500 ||
ast->chip == AST1180) {
dev->mode_config.max_width = 1920;
dev->mode_config.max_height = 2048;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 1ff596e..e4db1c72 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -271,7 +271,11 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct 
drm_display_mode *mod
 {
struct ast_private *ast = crtc->dev->dev_private;
u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 
0;
-   u16 temp;
+   u16 temp, precache = 0;
+
+   if ((ast->chip == AST2500) &&
+   (vbios_mode->enh_table->flags & AST2500PreCatchCRT))
+   precache = 40;
 
ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x11, 0x7f, 0x00);
 
@@ -297,12 +301,12 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, 
struct drm_display_mode *mod
jregAD |= 0x01;  /* HBE D[5] */
ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x03, 0xE0, (temp & 
0x1f));
 
-   temp = (mode->crtc_hsync_start &

[PATCH 8/9] drm/ast: POST code for the new AST2500

[Benjamin Herrenschmidt]

From: "Y.C. Chen" <yc_c...@aspeedtech.com>

This is used when the BMC isn't running any code and thus has
to be initialized by the host.

The code originates from Aspeed (Y.C. Chen) and has been cleaned
up for coding style purposes by BenH.

Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_dram_tables.h |  62 +
 drivers/gpu/drm/ast/ast_post.c| 410 +-
 2 files changed, 470 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dram_tables.h 
b/drivers/gpu/drm/ast/ast_dram_tables.h
index cc04539..1d9c4e7 100644
--- a/drivers/gpu/drm/ast/ast_dram_tables.h
+++ b/drivers/gpu/drm/ast/ast_dram_tables.h
@@ -141,4 +141,66 @@ static const struct ast_dramstruct 
ast2100_dram_table_data[] = {
{ 0x, 0x },
 };
 
+/*
+ * AST2500 DRAM settings modules
+ */
+#define REGTBL_NUM   17
+#define REGIDX_010   0
+#define REGIDX_014   1
+#define REGIDX_018   2
+#define REGIDX_020   3
+#define REGIDX_024   4
+#define REGIDX_02C   5
+#define REGIDX_030   6
+#define REGIDX_214   7
+#define REGIDX_2E0   8
+#define REGIDX_2E4   9
+#define REGIDX_2E8   10
+#define REGIDX_2EC   11
+#define REGIDX_2F0   12
+#define REGIDX_2F4   13
+#define REGIDX_2F8   14
+#define REGIDX_RFC   15
+#define REGIDX_PLL   16
+
+static const u32 ast2500_ddr3_1600_timing_table[REGTBL_NUM] = {
+   0x64604D38,  /* 0x010 */
+   0x29690599,  /* 0x014 */
+   0x0300,  /* 0x018 */
+   0x,  /* 0x020 */
+   0x,  /* 0x024 */
+   0x02181E70,  /* 0x02C */
+   0x0040,  /* 0x030 */
+   0x0024,  /* 0x214 */
+   0x02001300,  /* 0x2E0 */
+   0x0EA0,  /* 0x2E4 */
+   0x000E001B,  /* 0x2E8 */
+   0x35B8C105,  /* 0x2EC */
+   0x08090408,  /* 0x2F0 */
+   0x9B000800,  /* 0x2F4 */
+   0x0E400A00,  /* 0x2F8 */
+   0x9971452F,  /* tRFC  */
+   0x71C1   /* PLL   */
+};
+
+static const u32 ast2500_ddr4_1600_timing_table[REGTBL_NUM] = {
+   0x63604E37,  /* 0x010 */
+   0xE97AFA99,  /* 0x014 */
+   0x00019000,  /* 0x018 */
+   0x0800,  /* 0x020 */
+   0x0400,  /* 0x024 */
+   0x0410,  /* 0x02C */
+   0x0101,  /* 0x030 */
+   0x0024,  /* 0x214 */
+   0x03002900,  /* 0x2E0 */
+   0x0EA0,  /* 0x2E4 */
+   0x000E001C,  /* 0x2E8 */
+   0x35B8C106,  /* 0x2EC */
+   0x08080607,  /* 0x2F0 */
+   0x9B000900,  /* 0x2F4 */
+   0x0E400A00,  /* 0x2F8 */
+   0x99714545,  /* tRFC  */
+   0x71C1   /* PLL   */
+};
+
 #endif
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index a36a544..484138b 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -32,6 +32,7 @@
 #include "ast_dram_tables.h"
 
 static void ast_post_chip_2300(struct drm_device *dev);
+static void ast_post_chip_2500(struct drm_device *dev);
 
 void ast_enable_vga(struct drm_device *dev)
 {
@@ -82,7 +83,8 @@ ast_set_def_ext_reg(struct drm_device *dev)
for (i = 0x81; i <= 0x8f; i++)
ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
 
-   if (ast->chip == AST2300 || ast->chip == AST2400) {
+   if (ast->chip == AST2300 || ast->chip == AST2400 ||
+   ast->chip == AST2500) {
if (dev->pdev->revision >= 0x20)
ext_reg_info = extreginfo_ast2300;
else
@@ -106,7 +108,8 @@ ast_set_def_ext_reg(struct drm_device *dev)
 
/* Enable RAMDAC for A1 */
reg = 0x04;
-   if (ast->chip == AST2300 || ast->chip == AST2400)
+   if (ast->chip == AST2300 || ast->chip == AST2400 ||
+   ast->chip == AST2500)
reg |= 0x20;
ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);
 }
@@ -380,6 +383,8 @@ void ast_post_gpu(struct drm_device *dev)
ast_set_def_ext_reg(dev);
 
if (ast->config_mode == ast_use_p2a) {
+   if (ast->chip == AST2500)
+   ast_post_chip_2500(dev);
if (ast->chip == AST2300 || ast->chip == AST2400)
ast_post_chip_2300(dev);
 

[PATCH 4/9] drm/ast: Fix calculation of MCLK

[Benjamin Herrenschmidt]

Some braces were missing causing an incorrect calculation.

Y.C. Chen from Aspeed provided me with the right formula
which I tested on AST2400 and 2500.

The MCLK isn't currently used by the driver (it will eventually
to filter modes) so the issue isn't catastrophic.

Also make the printed value a bit more meaningful

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_main.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index b593a53..eb6ba3e 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -341,7 +341,7 @@ static int ast_get_dram_info(struct drm_device *dev)
div = 0x1;
break;
}
-   ast->mclk = ref_pll * (num + 2) / (denum + 2) * (div * 1000);
+   ast->mclk = ref_pll * (num + 2) / ((denum + 2) * (div * 1000));
return 0;
 }
 
@@ -485,7 +485,9 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
if (ret)
goto out_free;
ast->vram_size = ast_get_vram_info(dev);
-   DRM_INFO("dram %d %d %d %08x\n", ast->mclk, ast->dram_type, 
ast->dram_bus_width, ast->vram_size);
+   DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
+ast->mclk, ast->dram_type,
+ast->dram_bus_width, ast->vram_size);
}
 
if (need_post)
-- 
2.9.3

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

Re: [PATCH 02/12] drm/ast: Handle configuration without P2A bridge

[Benjamin Herrenschmidt]

On Fri, 2017-02-24 at 12:51 +1030, Joel Stanley wrote:
> 
> Are these properties supposed to repeat the prefix "ast,ast"?
> 
> We've chosen aspeed as the vendor prefix for Aspeed stuff.

Sent my reply too early ... so yes, I can change that, our FW hasn't
merge the FW side yet. I'll respin now.

> > +   if (mcr_scu_strap & 0x2000)
> 
> This bit confused me. Bit 13 of the strap (SCU70) is the SPI mode.

The register is actually "MCR170: AST2000 Backward Compatible SCU
Hardware Strapping Value"

> > +   ref_pll = 14318;
> > +   else
> > +   ref_pll = 12000;
> > +
> > +   denum = mcr_scu_mpll & 0x1f;
> > +   num = (mcr_scu_mpll & 0x3fe0) >> 5;
> > +   dsel = (mcr_scu_mpll & 0xc000) >> 14;
> 
> These calculations don't make sense for the ast2400 or ast2500.

They do if you look at this:

MCR120: AST2000 Backward Compatible SCU MPLL Parameter

It's not the SCU version of the register it's the MCU "copy" of it
that maintains some kind of legacy layout. Hence "mcr_scu" prefix
not "scu".

> > +   switch (dsel) {
> > +   case 3:
> > +   div = 0x4;
> > +   break;
> > +   case 2:
> > +   case 1:
> > +   div = 0x2;
> > +   break;
> > +   default:
> > +   div = 0x1;
> > +   break;
> > }
> > +   ast->mclk = ref_pll * (num + 2) / (denum + 2) * (div *
> > 1000);
> > return 0;
> >  }
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 2/12] drm/ast: Handle configuration without P2A bridge

[Benjamin Herrenschmidt]

The ast driver configures a window to enable access into BMC
memory space in order to read some configuration registers.

If this window is disabled, which it can be from the BMC side,
the ast driver can't function.

Closing this window is a necessity for security if a machine's
host side and BMC side are controlled by different parties;
i.e. a cloud provider offering machines "bare metal".

A recent patch went in to try to check if that window is open
but it does so by trying to access the registers in question
and testing if the result is 0x.

This method will trigger a PCIe error when the window is closed
which on some systems will be fatal (it will trigger an EEH
for example on POWER which will take out the device).

This patch improves this in two ways:

 - First, if the firmware has put properties in the device-tree
containing the relevant configuration information, we use these.

 - Otherwise, a bit in one of the SCU scratch registers (which
are readable via the VGA register space and writeable by the BMC)
will indicate if the BMC has closed the window. This bit has been
defined by Y.C Chen from Aspeed.

If the window is closed and the configuration isn't available from
the device-tree, some sane defaults are used. Those defaults are
hopefully sufficient for standard video modes used on a server.

Signed-off-by: Russell Currey <rus...@russell.cc>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
--

v2. [BenH]
- Reworked on top of Aspeed P2A patch
- Cleanup overall detection via a "config_mode" and log the
  selected mode for diagnostics purposes
- Add a property for the SCU straps

v3. [BenH]
- Moved the config mode detection to a separate functionn
- Add reading of SCU 0x40 D[12] to detect the window is
  closed as to not trigger a bus error by just "trying".
  (change provided by Y.C. Chen)
v4. [BenH]
- Only devices with the AST2000 PCI ID have a P2A bridge
- Update the P2A presence test to account for VGA only
  mode as provided by Y.C. Chen.
v5. [BenH]
- Fixup prefix of OF properties based on Joel Stanley
  review comments.
---
 drivers/gpu/drm/ast/ast_drv.h  |   6 +-
 drivers/gpu/drm/ast/ast_main.c | 264 +
 drivers/gpu/drm/ast/ast_post.c |   7 +-
 3 files changed, 168 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 7abda94..3bedcf7 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -113,7 +113,11 @@ struct ast_private {
struct ttm_bo_kmap_obj cache_kmap;
int next_cursor;
bool support_wide_screen;
-   bool DisableP2A;
+   enum {
+   ast_use_p2a,
+   ast_use_dt,
+   ast_use_defaults
+   } config_mode;
 
enum ast_tx_chip tx_chip_type;
u8 dp501_maxclk;
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 533e762..fb99762 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -62,13 +62,84 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
return ret;
 }
 
+static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
+{
+   struct device_node *np = dev->pdev->dev.of_node;
+   struct ast_private *ast = dev->dev_private;
+   uint32_t data, jregd0, jregd1;
+
+   /* Defaults */
+   ast->config_mode = ast_use_defaults;
+   *scu_rev = 0x;
+
+   /* Check if we have device-tree properties */
+   if (np && !of_property_read_u32(np, "aspeed,scu-revision-id",
+   scu_rev)) {
+   /* We do, disable P2A access */
+   ast->config_mode = ast_use_dt;
+   DRM_INFO("Using device-tree for configuration\n");
+   return;
+   }
+
+   /* Not all families have a P2A bridge */
+   if (dev->pdev->device != PCI_CHIP_AST2000)
+   return;
+
+   /*
+* The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
+* is disabled. We force using P2A if VGA only mode bit
+* is set D[7]
+*/
+   jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
+   jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
+   if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+   /* Double check it's actually working */
+   data = ast_read32(ast, 0xf004);
+   if (data != 0x) {
+   /* P2A works, grab silicon revision */
+   ast->config_mode = ast_use_p2a;
+
+   DRM_INFO("Using P2A bridge for configuration\n");
+
+   /* Read SCU7c (silicon revision register) */
+   ast_write32(ast, 0xf004, 0x1e6e);
+

[PATCH 09/12] drm/ast: Rename ast_init_dram_2300 to ast_post_chip_2300

[Benjamin Herrenschmidt]

The function does more than initializing the DRAM and in turns
calls other functions to do the actual init. This will keeping
things more consistent with the upcoming AST2500 POST code.

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_post.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index c55067c..561fd7d 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -31,7 +31,7 @@
 
 #include "ast_dram_tables.h"
 
-static void ast_init_dram_2300(struct drm_device *dev);
+static void ast_post_chip_2300(struct drm_device *dev);
 
 void ast_enable_vga(struct drm_device *dev)
 {
@@ -381,7 +381,7 @@ void ast_post_gpu(struct drm_device *dev)
 
if (ast->config_mode == ast_use_p2a) {
if (ast->chip == AST2300 || ast->chip == AST2400)
-   ast_init_dram_2300(dev);
+   ast_post_chip_2300(dev);
else
ast_init_dram_reg(dev);
 
@@ -1589,7 +1589,7 @@ static void ddr2_init(struct ast_private *ast, struct 
ast2300_dram_param *param)
 
 }
 
-static void ast_init_dram_2300(struct drm_device *dev)
+static void ast_post_chip_2300(struct drm_device *dev)
 {
struct ast_private *ast = dev->dev_private;
struct ast2300_dram_param param;
-- 
2.9.3

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

[PATCH 11/12] drm/ast: Fix test for VGA enabled

[Benjamin Herrenschmidt]

From: "Y.C. Chen" <yc_c...@aspeedtech.com>

(Get better description from Aspeed)

Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_post.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index c15f643..a5a7809 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -59,13 +59,9 @@ bool ast_is_vga_enabled(struct drm_device *dev)
/* TODO 1180 */
} else {
ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
-   if (ch) {
-   ast_open_key(ast);
-   ch = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 
0xb6, 0xff);
-   return ch & 0x04;
-   }
+   return !!(ch & 0x01);
}
-   return 0;
+   return false;
 }
 
 static const u8 extreginfo[] = { 0x0f, 0x04, 0x1c, 0xff };
-- 
2.9.3

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

Re: [PATCH 02/12] drm/ast: Handle configuration without P2A bridge

[Benjamin Herrenschmidt]

On Fri, 2017-02-24 at 12:51 +1030, Joel Stanley wrote:
> Are these properties supposed to repeat the prefix "ast,ast"?
> 
> We've chosen aspeed as the vendor prefix for Aspeed stuff.

Argh no, that's a typo... must have worked in my tests bcs
the defaults are fine. I'll update.

Cheers,
Ben.

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

Re: [PATCH 05/12] drm/ast: Fix calculation of MCLK

[Benjamin Herrenschmidt]

On Fri, 2017-02-24 at 12:54 +1030, Joel Stanley wrote:
> On Fri, Feb 24, 2017 at 9:23 AM, Benjamin Herrenschmidt
> <b...@kernel.crashing.org> wrote:
> > Some braces were missing causing an incorrect calculation.
> > 
> > Y.C. Chen from Aspeed provided me with the right formula
> > which I tested on AST2400 and 2500.
> 
> Y. C. Chen, can you point out this calculation in the programming
> guide?
> 
> All of the PLL calculations I can find in the ast2400 documentation
> are different to this one.

Different PLL register, see my other email. I've checked the result
of the calculation on our AST2500 and AST2400 machines.

Cheers,
Ben.

> Cheers,
> 
> Joel
> 
> > 
> > The MCLK isn't currently used by the driver (it will eventually
> > to filter modes) so the issue isn't catastrophic.
> > 
> > Also make the printed value a bit more meaningful
> > 
> > Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> > ---
> >  drivers/gpu/drm/ast/ast_main.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_main.c
> > b/drivers/gpu/drm/ast/ast_main.c
> > index 718c15b..d194af3 100644
> > --- a/drivers/gpu/drm/ast/ast_main.c
> > +++ b/drivers/gpu/drm/ast/ast_main.c
> > @@ -352,7 +352,7 @@ static int ast_get_dram_info(struct drm_device
> > *dev)
> > div = 0x1;
> > break;
> > }
> > -   ast->mclk = ref_pll * (num + 2) / (denum + 2) * (div *
> > 1000);
> > +   ast->mclk = ref_pll * (num + 2) / ((denum + 2) * (div *
> > 1000));
> > return 0;
> >  }
> > 
> > @@ -496,7 +496,9 @@ int ast_driver_load(struct drm_device *dev,
> > unsigned long flags)
> > if (ret)
> > goto out_free;
> > ast->vram_size = ast_get_vram_info(dev);
> > -   DRM_INFO("dram %d %d %d %08x\n", ast->mclk, ast-
> > >dram_type, ast->dram_bus_width, ast->vram_size);
> > +   DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d
> > size=%08x\n",
> > +ast->mclk, ast->dram_type,
> > +ast->dram_bus_width, ast->vram_size);
> > }
> > 
> > if (need_post)
> > --
> > 2.9.3
> > 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/12] drm/ast: POST code for the new AST2500

[Benjamin Herrenschmidt]

From: "Y.C. Chen" <yc_c...@aspeedtech.com>

This is used when the BMC isn't running any code and thus has
to be initialized by the host.

The code originates from Aspeed (Y.C. Chen) and has been cleaned
up for coding style purposes by BenH.

Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
--

v2. - Fix bug in ddr_test_2500 reported by Emil Velikov
- Rebase on updated mmc_test factoring patch
- Fix missing else statement in 2500 POST code
---
 drivers/gpu/drm/ast/ast_dram_tables.h |  62 +
 drivers/gpu/drm/ast/ast_post.c| 417 +-
 2 files changed, 476 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dram_tables.h 
b/drivers/gpu/drm/ast/ast_dram_tables.h
index cc04539..1d9c4e7 100644
--- a/drivers/gpu/drm/ast/ast_dram_tables.h
+++ b/drivers/gpu/drm/ast/ast_dram_tables.h
@@ -141,4 +141,66 @@ static const struct ast_dramstruct 
ast2100_dram_table_data[] = {
{ 0x, 0x },
 };
 
+/*
+ * AST2500 DRAM settings modules
+ */
+#define REGTBL_NUM   17
+#define REGIDX_010   0
+#define REGIDX_014   1
+#define REGIDX_018   2
+#define REGIDX_020   3
+#define REGIDX_024   4
+#define REGIDX_02C   5
+#define REGIDX_030   6
+#define REGIDX_214   7
+#define REGIDX_2E0   8
+#define REGIDX_2E4   9
+#define REGIDX_2E8   10
+#define REGIDX_2EC   11
+#define REGIDX_2F0   12
+#define REGIDX_2F4   13
+#define REGIDX_2F8   14
+#define REGIDX_RFC   15
+#define REGIDX_PLL   16
+
+static const u32 ast2500_ddr3_1600_timing_table[REGTBL_NUM] = {
+   0x64604D38,  /* 0x010 */
+   0x29690599,  /* 0x014 */
+   0x0300,  /* 0x018 */
+   0x,  /* 0x020 */
+   0x,  /* 0x024 */
+   0x02181E70,  /* 0x02C */
+   0x0040,  /* 0x030 */
+   0x0024,  /* 0x214 */
+   0x02001300,  /* 0x2E0 */
+   0x0EA0,  /* 0x2E4 */
+   0x000E001B,  /* 0x2E8 */
+   0x35B8C105,  /* 0x2EC */
+   0x08090408,  /* 0x2F0 */
+   0x9B000800,  /* 0x2F4 */
+   0x0E400A00,  /* 0x2F8 */
+   0x9971452F,  /* tRFC  */
+   0x71C1   /* PLL   */
+};
+
+static const u32 ast2500_ddr4_1600_timing_table[REGTBL_NUM] = {
+   0x63604E37,  /* 0x010 */
+   0xE97AFA99,  /* 0x014 */
+   0x00019000,  /* 0x018 */
+   0x0800,  /* 0x020 */
+   0x0400,  /* 0x024 */
+   0x0410,  /* 0x02C */
+   0x0101,  /* 0x030 */
+   0x0024,  /* 0x214 */
+   0x03002900,  /* 0x2E0 */
+   0x0EA0,  /* 0x2E4 */
+   0x000E001C,  /* 0x2E8 */
+   0x35B8C106,  /* 0x2EC */
+   0x08080607,  /* 0x2F0 */
+   0x9B000900,  /* 0x2F4 */
+   0x0E400A00,  /* 0x2F8 */
+   0x99714545,  /* tRFC  */
+   0x71C1   /* PLL   */
+};
+
 #endif
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 561fd7d..c15f643 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -32,6 +32,7 @@
 #include "ast_dram_tables.h"
 
 static void ast_post_chip_2300(struct drm_device *dev);
+static void ast_post_chip_2500(struct drm_device *dev);
 
 void ast_enable_vga(struct drm_device *dev)
 {
@@ -82,7 +83,8 @@ ast_set_def_ext_reg(struct drm_device *dev)
for (i = 0x81; i <= 0x9f; i++)
ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
 
-   if (ast->chip == AST2300 || ast->chip == AST2400) {
+   if (ast->chip == AST2300 || ast->chip == AST2400 ||
+   ast->chip == AST2500) {
if (dev->pdev->revision >= 0x20)
ext_reg_info = extreginfo_ast2300;
else
@@ -106,7 +108,8 @@ ast_set_def_ext_reg(struct drm_device *dev)
 
/* Enable RAMDAC for A1 */
reg = 0x04;
-   if (ast->chip == AST2300 || ast->chip == AST2400)
+   if (ast->chip == AST2300 || ast->chip == AST2400 ||
+   ast->chip == AST2500)
reg |= 0x20;
ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);
 }
@@ -380,7 +383,9 @@ void ast_post_gpu(struct drm_device *dev)
ast_set_def_ext_reg(dev);
 
if (ast->config_mode == ast_use_p2a) {
-   if (ast

[PATCH 01/12] drm/ast: Fix AST2400 POST failure without BMC FW or VBIOS

[Benjamin Herrenschmidt]

From: "Y.C. Chen" <yc_c...@aspeedtech.com>

The current POST code for the AST2300/2400 family doesn't work properly
if the chip hasn't been initialized previously by either the BMC own FW
or the VBIOS. This fixes it.

Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_post.c | 38 +++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 5331ee1..6c5391c 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -1638,12 +1638,44 @@ static void ast_init_dram_2300(struct drm_device *dev)
temp |= 0x73;
ast_write32(ast, 0x12008, temp);
 
+   param.dram_freq = 396;
param.dram_type = AST_DDR3;
+   temp = ast_mindwm(ast, 0x1e6e2070);
if (temp & 0x0100)
param.dram_type = AST_DDR2;
-   param.dram_chipid = ast->dram_type;
-   param.dram_freq = ast->mclk;
-   param.vram_size = ast->vram_size;
+switch (temp & 0x1800) {
+   case 0:
+   param.dram_chipid = AST_DRAM_512Mx16;
+   break;
+   default:
+   case 0x0800:
+   param.dram_chipid = AST_DRAM_1Gx16;
+   break;
+   case 0x1000:
+   param.dram_chipid = AST_DRAM_2Gx16;
+   break;
+   case 0x1800:
+   param.dram_chipid = AST_DRAM_4Gx16;
+   break;
+   }
+switch (temp & 0x0c) {
+default:
+   case 0x00:
+   param.vram_size = AST_VIDMEM_SIZE_8M;
+   break;
+
+   case 0x04:
+   param.vram_size = AST_VIDMEM_SIZE_16M;
+   break;
+
+   case 0x08:
+   param.vram_size = AST_VIDMEM_SIZE_32M;
+   break;
+
+   case 0x0c:
+   param.vram_size = AST_VIDMEM_SIZE_64M;
+   break;
+   }
 
if (param.dram_type == AST_DDR3) {
get_ddr3_info(ast, );
-- 
2.9.3

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

[PATCH 03/12] drm/ast: const'ify mode setting tables

[Benjamin Herrenschmidt]

And fix some comment alignment & space/tabs while at it

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_drv.h|   4 +-
 drivers/gpu/drm/ast/ast_mode.c   |   8 +--
 drivers/gpu/drm/ast/ast_tables.h | 106 +++
 3 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 3bedcf7..3fd9d6e 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -304,8 +304,8 @@ struct ast_vbios_dclk_info {
 };
 
 struct ast_vbios_mode_info {
-   struct ast_vbios_stdtable *std_table;
-   struct ast_vbios_enhtable *enh_table;
+   const struct ast_vbios_stdtable *std_table;
+   const struct ast_vbios_enhtable *enh_table;
 };
 
 extern int ast_mode_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index e26c98f..1ff596e 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -80,9 +80,9 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, 
struct drm_display_mo
 {
struct ast_private *ast = crtc->dev->dev_private;
u32 refresh_rate_index = 0, mode_id, color_index, refresh_rate;
+   const struct ast_vbios_enhtable *best = NULL;
u32 hborder, vborder;
bool check_sync;
-   struct ast_vbios_enhtable *best = NULL;
 
switch (crtc->primary->fb->bits_per_pixel) {
case 8:
@@ -146,7 +146,7 @@ static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, 
struct drm_display_mo
refresh_rate = drm_mode_vrefresh(mode);
check_sync = vbios_mode->enh_table->flags & WideScreenMode;
do {
-   struct ast_vbios_enhtable *loop = vbios_mode->enh_table;
+   const struct ast_vbios_enhtable *loop = vbios_mode->enh_table;
 
while (loop->refresh_rate != 0xff) {
if ((check_sync) &&
@@ -225,7 +225,7 @@ static void ast_set_std_reg(struct drm_crtc *crtc, struct 
drm_display_mode *mode
struct ast_vbios_mode_info *vbios_mode)
 {
struct ast_private *ast = crtc->dev->dev_private;
-   struct ast_vbios_stdtable *stdtable;
+   const struct ast_vbios_stdtable *stdtable;
u32 i;
u8 jreg;
 
@@ -381,7 +381,7 @@ static void ast_set_dclk_reg(struct drm_device *dev, struct 
drm_display_mode *mo
 struct ast_vbios_mode_info *vbios_mode)
 {
struct ast_private *ast = dev->dev_private;
-   struct ast_vbios_dclk_info *clk_info;
+   const struct ast_vbios_dclk_info *clk_info;
 
clk_info = _table[vbios_mode->enh_table->dclk_index];
 
diff --git a/drivers/gpu/drm/ast/ast_tables.h b/drivers/gpu/drm/ast/ast_tables.h
index 3608d5a..a4ddf90 100644
--- a/drivers/gpu/drm/ast/ast_tables.h
+++ b/drivers/gpu/drm/ast/ast_tables.h
@@ -78,37 +78,37 @@
 #define VCLK97_75  0x19
 #define VCLK118_25 0x1A
 
-static struct ast_vbios_dclk_info dclk_table[] = {
-   {0x2C, 0xE7, 0x03}, /* 00: 
VCLK25_175   */
-   {0x95, 0x62, 0x03}, /* 01: 
VCLK28_322   */
-   {0x67, 0x63, 0x01}, /* 02: VCLK31_5 
*/
-   {0x76, 0x63, 0x01}, /* 03: VCLK36   
*/
-   {0xEE, 0x67, 0x01}, /* 04: VCLK40   
*/
-   {0x82, 0x62, 0x01}, /* 05: VCLK49_5 
*/
-   {0xC6, 0x64, 0x01}, /* 06: VCLK50   
*/
-   {0x94, 0x62, 0x01}, /* 07: 
VCLK56_25*/
-   {0x80, 0x64, 0x00}, /* 08: VCLK65   
*/
-   {0x7B, 0x63, 0x00}, /* 09: VCLK75   
*/
-   {0x67, 0x62, 0x00}, /* 0A: 
VCLK78_75*/
-   {0x7C, 0x62, 0x00}, /* 0B: VCLK94_5 
*/
-   {0x8E, 0x62, 0x00}, /* 0C: VCLK108  
*/
-   {0x85, 0x24, 0x00}, /* 0D: VCLK135  
*/
-   {0x67, 0x22, 0x00}, /* 0E: 
VCLK157_5*/
-   {0x6A, 0x22, 0x00}, /* 0F: VCLK162  
*/
-   {0x4d, 0x4c, 0x80}, /* 10: VCLK154  
*/
-   {0xa7, 0x78, 0x80}, /* 11: VCLK83.5 
*/
-   {0x28, 0x49, 0x80}, /* 12: 
VCLK106.5*/
-   {0x37, 0x49, 0x80}, /* 13: 
VCLK146.25   */
-   {0x1f,

[PATCH 02/12] drm/ast: Handle configuration without P2A bridge

[Benjamin Herrenschmidt]

From: Russell Currey <rus...@russell.cc>

The ast driver configures a window to enable access into BMC
memory space in order to read some configuration registers.

If this window is disabled, which it can be from the BMC side,
the ast driver can't function.

Closing this window is a necessity for security if a machine's
host side and BMC side are controlled by different parties;
i.e. a cloud provider offering machines "bare metal".

A recent patch went in to try to check if that window is open
but it does so by trying to access the registers in question
and testing if the result is 0x.

This method will trigger a PCIe error when the window is closed
which on some systems will be fatal (it will trigger an EEH
for example on POWER which will take out the device).

This patch improves this in two ways:

 - First, if the firmware has put properties in the device-tree
containing the relevant configuration information, we use these.

 - Otherwise, a bit in one of the SCU scratch registers (which
are readable via the VGA register space and writeable by the BMC)
will indicate if the BMC has closed the window. This bit has been
defined by Y.C Chen from Aspeed.

If the window is closed and the configuration isn't available from
the device-tree, some sane defaults are used. Those defaults are
hopefully sufficient for standard video modes used on a server.

Signed-off-by: Russell Currey <rus...@russell.cc>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
--

v2. [BenH]
- Reworked on top of Aspeed P2A patch
- Cleanup overall detection via a "config_mode" and log the
  selected mode for diagnostics purposes
- Add a property for the SCU straps

v3. [BenH]
- Moved the config mode detection to a separate functionn
- Add reading of SCU 0x40 D[12] to detect the window is
  closed as to not trigger a bus error by just "trying".
  (change provided by Y.C. Chen)
v4. [BenH]
- Only devices with the AST2000 PCI ID have a P2A bridge
- Update the P2A presence test to account for VGA only
  mode as provided by Y.C. Chen.
---
 drivers/gpu/drm/ast/ast_drv.h  |   6 +-
 drivers/gpu/drm/ast/ast_main.c | 262 +
 drivers/gpu/drm/ast/ast_post.c |   7 +-
 3 files changed, 166 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 7abda94..3bedcf7 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -113,7 +113,11 @@ struct ast_private {
struct ttm_bo_kmap_obj cache_kmap;
int next_cursor;
bool support_wide_screen;
-   bool DisableP2A;
+   enum {
+   ast_use_p2a,
+   ast_use_dt,
+   ast_use_defaults
+   } config_mode;
 
enum ast_tx_chip tx_chip_type;
u8 dp501_maxclk;
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 533e762..36932a3 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -62,13 +62,83 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
return ret;
 }
 
+static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
+{
+   struct device_node *np = dev->pdev->dev.of_node;
+   struct ast_private *ast = dev->dev_private;
+   uint32_t data, jregd0, jregd1;
+
+   /* Defaults */
+   ast->config_mode = ast_use_defaults;
+   *scu_rev = 0x;
+
+   /* Check if we have device-tree properties */
+   if (np && !of_property_read_u32(np, "ast,scu-revision-id", scu_rev)) {
+   /* We do, disable P2A access */
+   ast->config_mode = ast_use_dt;
+   DRM_INFO("Using device-tree for configuration\n");
+   return;
+   }
+
+   /* Not all families have a P2A bridge */
+   if (dev->pdev->device != PCI_CHIP_AST2000)
+   return;
+
+   /*
+* The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
+* is disabled. We force using P2A if VGA only mode bit
+* is set D[7]
+*/
+   jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
+   jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
+   if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+   /* Double check it's actually working */
+   data = ast_read32(ast, 0xf004);
+   if (data != 0x) {
+   /* P2A works, grab silicon revision */
+   ast->config_mode = ast_use_p2a;
+
+   DRM_INFO("Using P2A bridge for configuration\n");
+
+   /* Read SCU7c (silicon revision register) */
+   ast_write32(ast, 0xf004, 0x1e6e);
+   ast_write32(ast, 0xf000, 0x1);
+   *scu_rev = ast_read32

[PATCH 04/12] drm/ast: Remove spurrious include

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 36932a3..718c15b 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -32,8 +32,6 @@
 #include 
 #include 
 
-#include "ast_dram_tables.h"
-
 void ast_set_index_reg_mask(struct ast_private *ast,
uint32_t base, uint8_t index,
uint8_t mask, uint8_t val)
-- 
2.9.3

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

[PATCH 12/12] drm/ast: Call open_key before enable_mmio in POST code

[Benjamin Herrenschmidt]

From: "Y.C. Chen" <yc_c...@aspeedtech.com>

open_key enables access the registers used by enable_mmio

Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_post.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index a5a7809..f7d4213 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -374,8 +374,8 @@ void ast_post_gpu(struct drm_device *dev)
pci_write_config_dword(ast->dev->pdev, 0x04, reg);
 
ast_enable_vga(dev);
-   ast_enable_mmio(dev);
ast_open_key(ast);
+   ast_enable_mmio(dev);
ast_set_def_ext_reg(dev);
 
if (ast->config_mode == ast_use_p2a) {
-- 
2.9.3

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

[PATCH 06/12] drm/ast: Base support for AST2500

[Benjamin Herrenschmidt]

From: "Y.C. Chen" <yc_c...@aspeedtech.com>

Add detection and mode setting updates for AST2500 generation chip,
code originally from Aspeed and slightly reworked for coding style
mostly by Ben. This doesn't contain the BMC DRAM POST code which
is in a separate patch.

Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---

v2. Add 800Mhz default mclk for AST2500
---
 drivers/gpu/drm/ast/ast_drv.h|  2 ++
 drivers/gpu/drm/ast/ast_main.c   | 32 +++---
 drivers/gpu/drm/ast/ast_mode.c   | 30 -
 drivers/gpu/drm/ast/ast_tables.h | 58 +---
 4 files changed, 103 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 3fd9d6e..d1c1d53 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -64,6 +64,7 @@ enum ast_chip {
AST2150,
AST2300,
AST2400,
+   AST2500,
AST1180,
 };
 
@@ -80,6 +81,7 @@ enum ast_tx_chip {
 #define AST_DRAM_1Gx32   3
 #define AST_DRAM_2Gx16   6
 #define AST_DRAM_4Gx16   7
+#define AST_DRAM_8Gx16   8
 
 struct ast_fbdev;
 
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index d194af3..f19669f 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -141,7 +141,10 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
ast->chip = AST1100;
DRM_INFO("AST 1180 detected\n");
} else {
-   if (dev->pdev->revision >= 0x30) {
+   if (dev->pdev->revision >= 0x40) {
+   ast->chip = AST2500;
+   DRM_INFO("AST 2500 detected\n");
+   } else if (dev->pdev->revision >= 0x30) {
ast->chip = AST2400;
DRM_INFO("AST 2400 detected\n");
} else if (dev->pdev->revision >= 0x20) {
@@ -195,6 +198,9 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
if (ast->chip == AST2400 &&
(scu_rev & 0x300) == 0x100) /* ast1400 */
ast->support_wide_screen = true;
+   if (ast->chip == AST2500 &&
+   scu_rev == 0x100)   /* ast2510 */
+   ast->support_wide_screen = true;
}
break;
}
@@ -289,7 +295,10 @@ static int ast_get_dram_info(struct drm_device *dev)
default:
ast->dram_bus_width = 16;
ast->dram_type = AST_DRAM_1Gx16;
-   ast->mclk = 396;
+   if (ast->chip == AST2500)
+   ast->mclk = 800;
+   else
+   ast->mclk = 396;
return 0;
}
 
@@ -298,7 +307,23 @@ static int ast_get_dram_info(struct drm_device *dev)
else
ast->dram_bus_width = 32;
 
-   if (ast->chip == AST2300 || ast->chip == AST2400) {
+   if (ast->chip == AST2500) {
+   switch (mcr_cfg & 0x03) {
+   case 0:
+   ast->dram_type = AST_DRAM_1Gx16;
+   break;
+   default:
+   case 1:
+   ast->dram_type = AST_DRAM_2Gx16;
+   break;
+   case 2:
+   ast->dram_type = AST_DRAM_4Gx16;
+   break;
+   case 3:
+   ast->dram_type = AST_DRAM_8Gx16;
+   break;
+   }
+   } else if (ast->chip == AST2300 || ast->chip == AST2400) {
switch (mcr_cfg & 0x03) {
case 0:
ast->dram_type = AST_DRAM_512Mx16;
@@ -521,6 +546,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
ast->chip == AST2200 ||
ast->chip == AST2300 ||
ast->chip == AST2400 ||
+   ast->chip == AST2500 ||
ast->chip == AST1180) {
dev->mode_config.max_width = 1920;
dev->mode_config.max_height = 2048;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 1ff596e..e4db1c72 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -271,7 +271,11 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct 
drm_display_mode *mod
 {
struct ast_private *ast = crtc->dev->dev_private;
u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 
0;
-   u16 temp;
+   u16 temp, precache = 0;
+
+   if ((ast->chip == AST2500) &&
+  

[PATCH 08/12] drm/ast: Factor mmc_test code in POST code

[Benjamin Herrenschmidt]

There's a some duplication for what's essentially copies of
two loops, so factor it. The upcoming AST2500 POST code adds
more of them. Also cleanup return types for the test functions,
most of them return a boolean, some return a u32.

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
--

v2. - Keep the split between the "test" and "test2" functions
  as they have a different exit condition in the loop and
  a different return type.
- Fix the return types accross the call chain
---
 drivers/gpu/drm/ast/ast_post.c | 82 --
 1 file changed, 31 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index e802450..c55067c 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -445,85 +445,65 @@ static const u32 pattern[8] = {
0x7C61D253
 };
 
-static int mmc_test_burst(struct ast_private *ast, u32 datagen)
+static bool mmc_test(struct ast_private *ast, u32 datagen, u8 test_ctl)
 {
u32 data, timeout;
 
ast_moutdwm(ast, 0x1e6e0070, 0x);
-   ast_moutdwm(ast, 0x1e6e0070, 0x00c1 | (datagen << 3));
+   ast_moutdwm(ast, 0x1e6e0070, (datagen << 3) | test_ctl);
timeout = 0;
do {
data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
-   if (data & 0x2000) {
-   return 0;
-   }
+   if (data & 0x2000)
+   return false;
if (++timeout > TIMEOUT) {
ast_moutdwm(ast, 0x1e6e0070, 0x);
-   return 0;
+   return false;
}
} while (!data);
-   ast_moutdwm(ast, 0x1e6e0070, 0x);
-   return 1;
+   ast_moutdwm(ast, 0x1e6e0070, 0x0);
+   return true;
 }
 
-static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
+static u32 mmc_test2(struct ast_private *ast, u32 datagen, u8 test_ctl)
 {
u32 data, timeout;
 
ast_moutdwm(ast, 0x1e6e0070, 0x);
-   ast_moutdwm(ast, 0x1e6e0070, 0x0041 | (datagen << 3));
+   ast_moutdwm(ast, 0x1e6e0070, (datagen << 3) | test_ctl);
timeout = 0;
do {
data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
if (++timeout > TIMEOUT) {
ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return -1;
+   return 0x;
}
} while (!data);
data = ast_mindwm(ast, 0x1e6e0078);
data = (data | (data >> 16)) & 0x;
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
+   ast_moutdwm(ast, 0x1e6e0070, 0x);
return data;
 }
 
-static int mmc_test_single(struct ast_private *ast, u32 datagen)
+
+static bool mmc_test_burst(struct ast_private *ast, u32 datagen)
 {
-   u32 data, timeout;
+   return mmc_test(ast, datagen, 0xc1);
+}
 
-   ast_moutdwm(ast, 0x1e6e0070, 0x);
-   ast_moutdwm(ast, 0x1e6e0070, 0x00c5 | (datagen << 3));
-   timeout = 0;
-   do {
-   data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
-   if (data & 0x2000)
-   return 0;
-   if (++timeout > TIMEOUT) {
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return 0;
-   }
-   } while (!data);
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return 1;
+static u32 mmc_test_burst2(struct ast_private *ast, u32 datagen)
+{
+   return mmc_test2(ast, datagen, 0x41);
 }
 
-static int mmc_test_single2(struct ast_private *ast, u32 datagen)
+static bool mmc_test_single(struct ast_private *ast, u32 datagen)
 {
-   u32 data, timeout;
+   return mmc_test(ast, datagen, 0xc5);
+}
 
-   ast_moutdwm(ast, 0x1e6e0070, 0x);
-   ast_moutdwm(ast, 0x1e6e0070, 0x0005 | (datagen << 3));
-   timeout = 0;
-   do {
-   data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
-   if (++timeout > TIMEOUT) {
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return -1;
-   }
-   } while (!data);
-   data = ast_mindwm(ast, 0x1e6e0078);
-   data = (data | (data >> 16)) & 0x;
-   ast_moutdwm(ast, 0x1e6e0070, 0x0);
-   return data;
+static u32 mmc_test_single2(struct ast_private *ast, u32 datagen)
+{
+   return mmc_test2(ast, datagen, 0x05);
 }
 
 static int cbr_test(struct ast_private *ast)
@@ -601,16 +581,16 @@ static u32 cbr_scan2(struct ast_private *ast)
return data2;
 }
 
-static u32 cbr_test3(struct ast_private *ast)
+static bool cbr_test3(struct ast_private *ast)
 {
if (!mmc_test_burst(ast, 0))
-   return 0;
+   return false;
if (!mmc_test_single(ast, 0))
- 

[PATCH 07/12] drm/ast: Fixed vram size incorrect issue on POWER

[Benjamin Herrenschmidt]

From: "Y.C. Chen" 

The default value of VGA scratch may incorrect.
Should initial h/w before get vram info.

Signed-off-by: Y.C. Chen 
---
 drivers/gpu/drm/ast/ast_main.c | 6 +++---
 drivers/gpu/drm/ast/ast_post.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f19669f..8684f3c 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -516,6 +516,9 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
 
ast_detect_chip(dev, _post);
 
+   if (need_post)
+   ast_post_gpu(dev);
+
if (ast->chip != AST1180) {
ret = ast_get_dram_info(dev);
if (ret)
@@ -526,9 +529,6 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
 ast->dram_bus_width, ast->vram_size);
}
 
-   if (need_post)
-   ast_post_gpu(dev);
-
ret = ast_mm_init(ast);
if (ret)
goto out_free;
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 64549ce..e802450 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -79,7 +79,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
const u8 *ext_reg_info;
 
/* reset scratch */
-   for (i = 0x81; i <= 0x8f; i++)
+   for (i = 0x81; i <= 0x9f; i++)
ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
 
if (ast->chip == AST2300 || ast->chip == AST2400) {
-- 
2.9.3

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

[PATCH 05/12] drm/ast: Fix calculation of MCLK

[Benjamin Herrenschmidt]

Some braces were missing causing an incorrect calculation.

Y.C. Chen from Aspeed provided me with the right formula
which I tested on AST2400 and 2500.

The MCLK isn't currently used by the driver (it will eventually
to filter modes) so the issue isn't catastrophic.

Also make the printed value a bit more meaningful

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_main.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 718c15b..d194af3 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -352,7 +352,7 @@ static int ast_get_dram_info(struct drm_device *dev)
div = 0x1;
break;
}
-   ast->mclk = ref_pll * (num + 2) / (denum + 2) * (div * 1000);
+   ast->mclk = ref_pll * (num + 2) / ((denum + 2) * (div * 1000));
return 0;
 }
 
@@ -496,7 +496,9 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
if (ret)
goto out_free;
ast->vram_size = ast_get_vram_info(dev);
-   DRM_INFO("dram %d %d %d %08x\n", ast->mclk, ast->dram_type, 
ast->dram_bus_width, ast->vram_size);
+   DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
+ast->mclk, ast->dram_type,
+ast->dram_bus_width, ast->vram_size);
}
 
if (need_post)
-- 
2.9.3

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

Re: [PATCH 01/12] drm/ast: Fix AST2400 POST failure without BMC FW or VBIOS

[Benjamin Herrenschmidt]

Note: The whole series with the fixed cset comment for patch 11
can be also found there:

https://github.com/ozbenh/linux-ast/commits/master

Cheers,
Ben.

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

Re: [PATCH 11/12] drm/ast: Fix test for VGA enabled

[Benjamin Herrenschmidt]

On Fri, 2017-02-24 at 09:53 +1100, Benjamin Herrenschmidt wrote:
> From: "Y.C. Chen" <yc_c...@aspeedtech.com>
> 
> (Get better description from Aspeed)

And this should have been:

<<
The test to see if VGA was already enabled is doing an unnecessary
second test from a register that may or may not have been initialized
to a valid value. Remove it.
>>

If you prefer you can find the whole thing (already fixed up)
at g...@github.com:ozbenh/linux-ast.git

Cheers,
Ben.

> Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> ---
>  drivers/gpu/drm/ast/ast_post.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_post.c
> b/drivers/gpu/drm/ast/ast_post.c
> index c15f643..a5a7809 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -59,13 +59,9 @@ bool ast_is_vga_enabled(struct drm_device *dev)
>   /* TODO 1180 */
>   } else {
>   ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
> - if (ch) {
> - ast_open_key(ast);
> - ch = ast_get_index_reg_mask(ast,
> AST_IO_CRTC_PORT, 0xb6, 0xff);
> - return ch & 0x04;
> - }
> + return !!(ch & 0x01);
>   }
> - return 0;
> + return false;
>  }
>  
>  static const u8 extreginfo[] = { 0x0f, 0x04, 0x1c, 0xff };

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

Re: [PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Thu, 2017-02-16 at 20:09 +1100, Benjamin Herrenschmidt wrote:
> From: "Y.C. Chen" <yc_c...@aspeedtech.com>
> 
> Add detection and POST code for AST2500 generation chip,
> code originally from Aspeed and slightly reworked for
> coding style mostly by Ben.

I just noticed there's still a bunch of minor checkpatch warnings,
I'll do a new spin tomorrow but without code changes.

Y.C. Can you still test this one please to make sure I didn't break
POST ? :-)

Cheers,
Ben.

> Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> ---
> v2. [BenH]
>   - Coding style fixes
>   - Add timeout to main DRAM init loop
>   - Improve boot time display of RAM info
> 
> Y.C. Please check that I didn't break POST as I haven't manage to
> test
> that (we can't boot our machines if the BMC isn't already POSTed).
> 
> ---
>  drivers/gpu/drm/ast/ast_dram_tables.h |  62 +
>  drivers/gpu/drm/ast/ast_drv.h |   3 +
>  drivers/gpu/drm/ast/ast_main.c|  32 ++-
>  drivers/gpu/drm/ast/ast_mode.c|  25 +-
>  drivers/gpu/drm/ast/ast_post.c| 491
> ++
>  drivers/gpu/drm/ast/ast_tables.h  |  57 +++-
>  6 files changed, 586 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dram_tables.h
> b/drivers/gpu/drm/ast/ast_dram_tables.h
> index cc04539..02265b5 100644
> --- a/drivers/gpu/drm/ast/ast_dram_tables.h
> +++ b/drivers/gpu/drm/ast/ast_dram_tables.h
> @@ -141,4 +141,66 @@ static const struct ast_dramstruct
> ast2100_dram_table_data[] = {
>   { 0x, 0x },
>  };
>  
> +/*
> + * AST2500 DRAM settings modules
> + */
> +#define REGTBL_NUM   17
> +#define REGIDX_010   0
> +#define REGIDX_014   1
> +#define REGIDX_018   2
> +#define REGIDX_020   3
> +#define REGIDX_024   4
> +#define REGIDX_02C   5
> +#define REGIDX_030   6
> +#define REGIDX_214   7
> +#define REGIDX_2E0   8
> +#define REGIDX_2E4   9
> +#define REGIDX_2E8   10
> +#define REGIDX_2EC   11
> +#define REGIDX_2F0   12
> +#define REGIDX_2F4   13
> +#define REGIDX_2F8   14
> +#define REGIDX_RFC   15
> +#define REGIDX_PLL   16
> +
> +static u32 ast2500_ddr3_1600_timing_table[REGTBL_NUM] = {
> + 0x64604D38,  /* 0x010 */
> + 0x29690599,  /* 0x014 */
> + 0x0300,  /* 0x018 */
> + 0x,  /* 0x020 */
> + 0x,  /* 0x024 */
> + 0x02181E70,  /* 0x02C */
> + 0x0040,  /* 0x030 */
> + 0x0024,  /* 0x214 */
> + 0x02001300,  /* 0x2E0 */
> + 0x0EA0,  /* 0x2E4 */
> + 0x000E001B,  /* 0x2E8 */
> + 0x35B8C105,  /* 0x2EC */
> + 0x08090408,  /* 0x2F0 */
> + 0x9B000800,  /* 0x2F4 */
> + 0x0E400A00,  /* 0x2F8 */
> + 0x9971452F,  /* tRFC  */
> + 0x71C1   /* PLL   */
> +};
> +
> +static u32 ast2500_ddr4_1600_timing_table[REGTBL_NUM] = {
> + 0x63604E37,  /* 0x010 */
> + 0xE97AFA99,  /* 0x014 */
> + 0x00019000,  /* 0x018 */
> + 0x0800,  /* 0x020 */
> + 0x0400,  /* 0x024 */
> + 0x0410,  /* 0x02C */
> + 0x0101,  /* 0x030 */
> + 0x0024,  /* 0x214 */
> + 0x03002900,  /* 0x2E0 */
> + 0x0EA0,  /* 0x2E4 */
> + 0x000E001C,  /* 0x2E8 */
> + 0x35B8C106,  /* 0x2EC */
> + 0x08080607,  /* 0x2F0 */
> + 0x9B000900,  /* 0x2F4 */
> + 0x0E400A00,  /* 0x2F8 */
> + 0x99714545,  /* tRFC  */
> + 0x71C1   /* PLL   */
> +};
> +
>  #endif
> diff --git a/drivers/gpu/drm/ast/ast_drv.h
> b/drivers/gpu/drm/ast/ast_drv.h
> index 3bedcf7..2db97e2 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -64,6 +64,7 @@ enum ast_chip {
>   AST2150,
>   AST2300,
>   AST2400,
> + AST2500,
>   AST1180,
>  };
>  
> @@ -80,6 +81,7 @@ enum ast_tx_chip {
>  #define AST_DRAM_1Gx32   3
>  #define AST_DRAM_2Gx16   6
>  #define AST_DRAM_4Gx16   7
> +#define AST_DRAM_8Gx16   8
>  
>  struct a

[PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

From: "Y.C. Chen" <yc_c...@aspeedtech.com>

Add detection and POST code for AST2500 generation chip,
code originally from Aspeed and slightly reworked for
coding style mostly by Ben.

Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
v2. [BenH]
  - Coding style fixes
  - Add timeout to main DRAM init loop
  - Improve boot time display of RAM info

Y.C. Please check that I didn't break POST as I haven't manage to test
that (we can't boot our machines if the BMC isn't already POSTed).

---
 drivers/gpu/drm/ast/ast_dram_tables.h |  62 +
 drivers/gpu/drm/ast/ast_drv.h |   3 +
 drivers/gpu/drm/ast/ast_main.c|  32 ++-
 drivers/gpu/drm/ast/ast_mode.c|  25 +-
 drivers/gpu/drm/ast/ast_post.c| 491 ++
 drivers/gpu/drm/ast/ast_tables.h  |  57 +++-
 6 files changed, 586 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dram_tables.h 
b/drivers/gpu/drm/ast/ast_dram_tables.h
index cc04539..02265b5 100644
--- a/drivers/gpu/drm/ast/ast_dram_tables.h
+++ b/drivers/gpu/drm/ast/ast_dram_tables.h
@@ -141,4 +141,66 @@ static const struct ast_dramstruct 
ast2100_dram_table_data[] = {
{ 0x, 0x },
 };
 
+/*
+ * AST2500 DRAM settings modules
+ */
+#define REGTBL_NUM   17
+#define REGIDX_010   0
+#define REGIDX_014   1
+#define REGIDX_018   2
+#define REGIDX_020   3
+#define REGIDX_024   4
+#define REGIDX_02C   5
+#define REGIDX_030   6
+#define REGIDX_214   7
+#define REGIDX_2E0   8
+#define REGIDX_2E4   9
+#define REGIDX_2E8   10
+#define REGIDX_2EC   11
+#define REGIDX_2F0   12
+#define REGIDX_2F4   13
+#define REGIDX_2F8   14
+#define REGIDX_RFC   15
+#define REGIDX_PLL   16
+
+static u32 ast2500_ddr3_1600_timing_table[REGTBL_NUM] = {
+   0x64604D38,  /* 0x010 */
+   0x29690599,  /* 0x014 */
+   0x0300,  /* 0x018 */
+   0x,  /* 0x020 */
+   0x,  /* 0x024 */
+   0x02181E70,  /* 0x02C */
+   0x0040,  /* 0x030 */
+   0x0024,  /* 0x214 */
+   0x02001300,  /* 0x2E0 */
+   0x0EA0,  /* 0x2E4 */
+   0x000E001B,  /* 0x2E8 */
+   0x35B8C105,  /* 0x2EC */
+   0x08090408,  /* 0x2F0 */
+   0x9B000800,  /* 0x2F4 */
+   0x0E400A00,  /* 0x2F8 */
+   0x9971452F,  /* tRFC  */
+   0x71C1   /* PLL   */
+};
+
+static u32 ast2500_ddr4_1600_timing_table[REGTBL_NUM] = {
+   0x63604E37,  /* 0x010 */
+   0xE97AFA99,  /* 0x014 */
+   0x00019000,  /* 0x018 */
+   0x0800,  /* 0x020 */
+   0x0400,  /* 0x024 */
+   0x0410,  /* 0x02C */
+   0x0101,  /* 0x030 */
+   0x0024,  /* 0x214 */
+   0x03002900,  /* 0x2E0 */
+   0x0EA0,  /* 0x2E4 */
+   0x000E001C,  /* 0x2E8 */
+   0x35B8C106,  /* 0x2EC */
+   0x08080607,  /* 0x2F0 */
+   0x9B000900,  /* 0x2F4 */
+   0x0E400A00,  /* 0x2F8 */
+   0x99714545,  /* tRFC  */
+   0x71C1   /* PLL   */
+};
+
 #endif
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 3bedcf7..2db97e2 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -64,6 +64,7 @@ enum ast_chip {
AST2150,
AST2300,
AST2400,
+   AST2500,
AST1180,
 };
 
@@ -80,6 +81,7 @@ enum ast_tx_chip {
 #define AST_DRAM_1Gx32   3
 #define AST_DRAM_2Gx16   6
 #define AST_DRAM_4Gx16   7
+#define AST_DRAM_8Gx16   8
 
 struct ast_fbdev;
 
@@ -397,6 +399,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
 void ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_private *ast, u32 r);
 void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
+
 /* ast dp501 */
 int ast_load_dp501_microcode(struct drm_device *dev);
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index d42a4f5..60f23cc 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -32,8 +32,6 @@
 #include 
 #include 
 
-#include "ast_dram_tables.h"
-
 void ast_set_index_reg_mask(struct ast_private *ast,
uint32_t base, uint8_t index,
uint8_t mask, uint8_

[PATCH 1/2] drm/ast: Support reading configuration values

[Benjamin Herrenschmidt]

From: Russell Currey <rus...@russell.cc>

The ast driver configures a window to enable access into BMC memory space
in order to read some configuration registers.  If this window is
disabled, which it can be from the BMC side, the ast driver can't
function.  Closing this window is a necessity for security if a machine's
host side and BMC side are controlled by different parties; i.e. a cloud
provider offering machines "bare metal".

A recent patch from Aspeed tries to detect whether that window is
enabled and uses default values if not. Unfortunately this won't work
on systems such as POWER machines that have strong PCIe error handling
as the attempt at detection will cause a PCIe abort if the window is
closed.

To work around this, enable reading these configuration values from the
device tree instead of through the window into BMC memory if the
corresponding DT properties are available. If not, the driver defaults
to the existing test which i cleaned up a little bit.

Signed-off-by: Russell Currey <rus...@russell.cc>
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
--
v2. [BenH]
- Reworked on top of Aspeed P2A patch
- Cleanup overall detection via a "config_mode" and log the
  selected mode for diagnostics purposes
- Add a property for the SCU straps
---
 drivers/gpu/drm/ast/ast_drv.h  |   6 +-
 drivers/gpu/drm/ast/ast_main.c | 208 -
 drivers/gpu/drm/ast/ast_post.c |   7 +-
 3 files changed, 127 insertions(+), 94 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 7abda94..3bedcf7 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -113,7 +113,11 @@ struct ast_private {
struct ttm_bo_kmap_obj cache_kmap;
int next_cursor;
bool support_wide_screen;
-   bool DisableP2A;
+   enum {
+   ast_use_p2a,
+   ast_use_dt,
+   ast_use_defaults
+   } config_mode;
 
enum ast_tx_chip tx_chip_type;
u8 dp501_maxclk;
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 533e762..d42a4f5 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -65,10 +65,43 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
 
 static int ast_detect_chip(struct drm_device *dev, bool *need_post)
 {
+   struct device_node *np = dev->pdev->dev.of_node;
struct ast_private *ast = dev->dev_private;
-   uint32_t data, jreg;
+   uint32_t data, jreg, scu_rev;
ast_open_key(ast);
 
+
+   /* Check if we have device-tree properties */
+   if (np && !of_property_read_u32(np, "ast,scu-revision-id", _rev)) {
+   /* We do, disable P2A access */
+   ast->config_mode = ast_use_dt;
+   DRM_INFO("Using device-tree for configuration\n");
+   } else {
+   /*
+* Check if P2A access works. This will trigger a PCIe abort
+* if it doesn't so it's not recommended on platforms where
+* this is a fatal error. Those should use the device-tree
+* or Aspeed should come up with a different mechanism.
+*/
+   data = ast_read32(ast, 0xf004);
+   if (data == 0x) {
+   ast->config_mode = ast_use_defaults;
+   scu_rev = 0x;
+   DRM_INFO("P2A bridge disabled, using default"
+" configuration\n");
+   } else {
+   /* P2A works, grab silicon revision */
+   ast->config_mode = ast_use_p2a;
+
+   DRM_INFO("Using P2A bridge for configuration\n");
+
+   /* Read SCU7c (silicon revision register) */
+   ast_write32(ast, 0xf004, 0x1e6e);
+   ast_write32(ast, 0xf000, 0x1);
+   scu_rev = ast_read32(ast, 0x1207c);
+   }
+   }
+
if (dev->pdev->device == PCI_CHIP_AST1180) {
ast->chip = AST1100;
DRM_INFO("AST 1180 detected\n");
@@ -80,12 +113,7 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
ast->chip = AST2300;
DRM_INFO("AST 2300 detected\n");
} else if (dev->pdev->revision >= 0x10) {
-   uint32_t data;
-   ast_write32(ast, 0xf004, 0x1e6e);
-   ast_write32(ast, 0xf000, 0x1);
-
-   data = ast_read32(ast, 0x1207c);
-   switch (data & 0x0300) {
+   switch (scu_rev & 0x0300) {
case 0x0200:

[PATCH] drm/ast: Fix mclk calculation

[Benjamin Herrenschmidt]

The very large values observed were the result of a bug in the
calculation. Y.C. Chen gave me the right formula so here is
a patch fixing it.

Thankfully nothing seem to use that unless you try to POST the
chip.

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
--

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 60f23cc..3b87877 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -350,7 +350,7 @@ static int ast_get_dram_info(struct drm_device *dev)
div = 0x1;
break;
}
-   ast->mclk = ref_pll * (num + 2) / (denum + 2) * (div * 1000);
+   ast->mclk = ref_pll * (num + 2) / ((denum + 2) * (div * 1000));
return 0;
 }
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Thu, 2017-02-16 at 17:33 +, Emil Velikov wrote:
> On 16 February 2017 at 09:09, Benjamin Herrenschmidt
> <b...@kernel.crashing.org> wrote:
> > From: "Y.C. Chen" <yc_c...@aspeedtech.com>
> > 
> > Add detection and POST code for AST2500 generation chip,
> > code originally from Aspeed and slightly reworked for
> > coding style mostly by Ben.
> > 
> > Signed-off-by: Y.C. Chen <yc_c...@aspeedtech.com>
> > Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> > ---
> > v2. [BenH]
> >   - Coding style fixes
> >   - Add timeout to main DRAM init loop
> >   - Improve boot time display of RAM info
> > 
> > Y.C. Please check that I didn't break POST as I haven't manage to
> > test
> > that (we can't boot our machines if the BMC isn't already POSTed).
> > 
> 
> Hi Ben,
> 
> Barring the checkpatch.pl warnings/errors that you've spotted there's
> a few other comments below.
> I'm not familiar with the hardware, so it's just things that strike
> me
> as odd ;-)

Heh ok. I don't want to change that POST code too much as I'm not
equipped to test it, but I'll have a look later today.

Thanks,
Ben.

> 
> > +/*
> > + * AST2500 DRAM settings modules
> > + */
> > +#define REGTBL_NUM   17
> > +#define REGIDX_010   0
> > +#define REGIDX_014   1
> > +#define REGIDX_018   2
> > +#define REGIDX_020   3
> > +#define REGIDX_024   4
> > +#define REGIDX_02C   5
> > +#define REGIDX_030   6
> > +#define REGIDX_214   7
> > +#define REGIDX_2E0   8
> > +#define REGIDX_2E4   9
> > +#define REGIDX_2E8   10
> > +#define REGIDX_2EC   11
> > +#define REGIDX_2F0   12
> > +#define REGIDX_2F4   13
> > +#define REGIDX_2F8   14
> > +#define REGIDX_RFC   15
> > +#define REGIDX_PLL   16
> 
> These are used to address the correct value in the array, Worth using
> C99 initailizer to ensure that things end in the right place ?
> IIRC there was some security related GCC plugin which can fuzz the
> order of array(?) and/or struct members ?
> 
> > +
> > +static u32 ast2500_ddr3_1600_timing_table[REGTBL_NUM] = {
> > +static u32 ast2500_ddr4_1600_timing_table[REGTBL_NUM] = {
> 
> These two are constant data, although you'll need to annotate the
> users as well.
> 
> 
> > --- a/drivers/gpu/drm/ast/ast_main.c
> > +++ b/drivers/gpu/drm/ast/ast_main.c
> > @@ -32,8 +32,6 @@
> >  #include 
> >  #include 
> > 
> > -#include "ast_dram_tables.h"
> > -
> 
> Unrelated change ?
> 
> 
> > -   DRM_INFO("dram %d %d %d %08x\n", ast->mclk, ast-
> > >dram_type, ast->dram_bus_width, ast->vram_size);
> > +   DRM_INFO("dram MCLK=%u type=%d bus_width=%d
> > size=%08x\n",
> > +ast->mclk, ast->dram_type, ast-
> > >dram_bus_width, ast->vram_size);
> 
> Unrelated change ?
> 
> 
> >  static void ast_set_ext_reg(struct drm_crtc *crtc, struct
> > drm_display_mode *mode,
> > @@ -421,7 +432,7 @@ static void ast_set_ext_reg(struct drm_crtc
> > *crtc, struct drm_display_mode *mode
> > ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa8, 0xfd,
> > jregA8);
> > 
> > /* Set Threshold */
> > -   if (ast->chip == AST2300 || ast->chip == AST2400) {
> > +   if (ast->chip == AST2300 || ast->chip == AST2400 || ast-
> > >chip == AST2500) {
> > ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7,
> > 0x78);
> > ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6,
> > 0x60);
> > } else if (ast->chip == AST2100 ||
> > @@ -794,7 +805,7 @@ static int ast_mode_valid(struct drm_connector
> > *connector,
> > if ((mode->hdisplay == 1600) && (mode->vdisplay ==
> > 900))
> > return MODE_OK;
> > 
> > -   if ((ast->chip == AST2100) || (ast->chip ==
> > AST2200) || (ast->chip == AST2300) || (ast->chip == AST2400) ||
> > (ast->chip == AST1180)) {
> > +   if ((ast->chip == AST2100) || (ast->chip ==
> > AST2200) || (ast->chip == AST2300) || (ast->chip == AST2400) ||
> > (ast->chip == AST2500) || (ast->chip == AST1180)) {
> 
> 178 columns is getting a bit too much.
> 
> 
> > -static int mmc_test_burst(struct ast_private *ast, u32 

Re: [PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Thu, 2017-02-16 at 17:33 +, Emil Velikov wrote:
> The above seems like a _lot_ of unrelated rework. Keep the
> refactoring
> separate ?
> New code seems to assume that only(?) -1 is returned on error, yet we
> do < 0.
> This will come to bite you/others as the tests are extended/reworked.

Not sure what you mean.

I made the error return consistent yes, using -1.

< 0 is a reasonable way to test this (and probably marginally more
efficient than comparing against -1) , the "good" values are always
in the range 0.. .

I did the refactoring in that patch because we have 4 functions doing
roughly the same thing and the 2500 patch was adding 2 more.

I could do a first patch just changing the existing code I suppose...

Ben.

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

Re: [PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Thu, 2017-02-16 at 17:33 +, Emil Velikov wrote:
> I realise that there's likely no documentation, yet repeating the
> same
> magic numbers multiple times is a bit meh.

This is all Aspeed original code. I merely fixed the coding style ;-)

The other POST functions for the other chip gens are the same in that
regard.

That said I do have the doc, I could create constants for everything
but I really don't have that much time and we are in an emergency toget that 
into distros...

What I might do is do a separate patch later that replaces a bunch
of the hard coded values with properly defined constants.

Cheers,
Ben.


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

Re: [PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Thu, 2017-02-16 at 17:33 +, Emil Velikov wrote:
> "Water is wet" type of comment. Worth mentioning why it's so large -
> mentioned in the documentation, experimental result, other ?
> Same suggestion goes for the other mdelay(100) instances.

Ah I removed most of those useless comments, I must have missed
that one. As for why 100ms, that's aspeed code. The init sequence
per-se isn't documented well, so I assume they know what they are doing
:-)

Keep in mind that this code is almost never used. It's only used
if the BMC is running no code at all, to "remotely" initialize its
memory controller.

Cheers,
Ben.

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

Re: [PATCH 2/2] drm/ast: Support AST2500

[Benjamin Herrenschmidt]

On Thu, 2017-02-16 at 17:33 +, Emil Velikov wrote:
> > +static bool ast_dram_init_2500(struct ast_private *ast)
> > +{
> > +   u32 data;
> > +   u32 max_tries = 5;
> > +
> > +   do {
> > +   if (max_tries-- == 0)
> > +   return false;
> > +   set_mpll_2500(ast);
> > +   reset_mmc_2500(ast);
> > +   ddr_init_common_2500(ast);
> > +
> > +   data = ast_mindwm(ast, 0x1E6E2070);
> > +   if (data & 0x0100)
> > +   ddr4_init_2500(ast, ast2500_ddr4_1600_timing_table);
> > +   else
> > +   ddr3_init_2500(ast, ast2500_ddr3_1600_timing_table);
> > +   } while (!ddr_test_2500(ast));
> > +
> > +   ast_moutdwm(ast, 0x1E6E2040, ast_mindwm(ast, 0x1E6E2040) | 0x41);
> > +
> > +   /* Patch code */
> > +   data = ast_mindwm(ast, 0x1E6E200C) & 0xF9FF;
> > +   ast_moutdwm(ast, 0x1E6E200C, data | 0x1000);
> > +
> > +   return true;
> 
> Drop the return type - function always returns true ?
> I think there were a few other functions that could do the same.

It's not. I added a timeout with a return false ;-)

Cheers,
Ben.

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

[PATCH 3/9] drm/ast: Remove spurrious include

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
 drivers/gpu/drm/ast/ast_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 823c68f..b593a53 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -32,8 +32,6 @@
 #include 
 #include 
 
-#include "ast_dram_tables.h"
-
 void ast_set_index_reg_mask(struct ast_private *ast,
uint32_t base, uint8_t index,
uint8_t mask, uint8_t val)
-- 
2.9.3

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

[PATCH 9/9] drm/ast: Fixed vram size incorrect issue on POWER

[Benjamin Herrenschmidt]

From: "Y.C. Chen" 

The default value of VGA scratch may incorrect.
Should initial h/w before get vram info.

Signed-off-by: Y.C. Chen 
---
 drivers/gpu/drm/ast/ast_main.c | 6 +++---
 drivers/gpu/drm/ast/ast_post.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f9128b9..839e456 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -502,6 +502,9 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
 
ast_detect_chip(dev, _post);
 
+   if (need_post)
+   ast_post_gpu(dev);
+
if (ast->chip != AST1180) {
ret = ast_get_dram_info(dev);
if (ret)
@@ -512,9 +515,6 @@ int ast_driver_load(struct drm_device *dev, unsigned long 
flags)
 ast->dram_bus_width, ast->vram_size);
}
 
-   if (need_post)
-   ast_post_gpu(dev);
-
ret = ast_mm_init(ast);
if (ret)
goto out_free;
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 484138b..9f9 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -80,7 +80,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
const u8 *ext_reg_info;
 
/* reset scratch */
-   for (i = 0x81; i <= 0x8f; i++)
+   for (i = 0x81; i <= 0x9f; i++)
ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
 
if (ast->chip == AST2300 || ast->chip == AST2400 ||
-- 
2.9.3

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

Re: [PATCH 1/9] drm/ast: Handle configuration without P2A bridge

[Benjamin Herrenschmidt]

On Fri, 2017-02-17 at 16:32 +1100, Benjamin Herrenschmidt wrote:
> The ast driver configures a window to enable access into BMC
> memory space in order to read some configuration registers.

Aspeed suggested a couple of refinements for some chipsets,
so I'll respin either this week-end or monday.

> If this window is disabled, which it can be from the BMC side,
> the ast driver can't function.
> 
> Closing this window is a necessity for security if a machine's
> host side and BMC side are controlled by different parties;
> i.e. a cloud provider offering machines "bare metal".
> 
> A recent patch went in to try to check if that window is open
> but it does so by trying to access the registers in question
> and testing if the result is 0x.
> 
> This method will trigger a PCIe error when the window is closed
> which on some systems will be fatal (it will trigger an EEH
> for example on POWER which will take out the device).
> 
> This patch improves this in two ways:
> 
>  - First, if the firmware has put properties in the device-tree
> containing the relevant configuration information, we use these.
> 
>  - Otherwise, a bit in one of the SCU scratch registers (which
> are readable via the VGA register space and writeable by the BMC)
> will indicate if the BMC has closed the window. This bit has been
> defined by Y.C Chen from Aspeed.
> 
> If the window is closed and the configuration isn't available from
> the device-tree, some sane defaults are used. Those defaults are
> hopefully sufficient for standard video modes used on a server.
> 
> Signed-off-by: Russell Currey <rus...@russell.cc>
> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> --
> 
> v2. [BenH]
> - Reworked on top of Aspeed P2A patch
> - Cleanup overall detection via a "config_mode" and log the
>   selected mode for diagnostics purposes
> - Add a property for the SCU straps
> 
> v3. [BenH]
> - Moved the config mode detection to a separate functionn
> - Add reading of SCU 0x40 D[12] to detect the window is
>   closed as to not trigger a bus error by just "trying".
>   (change provided by Y.C. Chen)
> ---
>  drivers/gpu/drm/ast/ast_drv.h  |   6 +-
>  drivers/gpu/drm/ast/ast_main.c | 223 +
> 
>  drivers/gpu/drm/ast/ast_post.c |   7 +-
>  3 files changed, 141 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h
> b/drivers/gpu/drm/ast/ast_drv.h
> index 7abda94..3bedcf7 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -113,7 +113,11 @@ struct ast_private {
>   struct ttm_bo_kmap_obj cache_kmap;
>   int next_cursor;
>   bool support_wide_screen;
> - bool DisableP2A;
> + enum {
> + ast_use_p2a,
> + ast_use_dt,
> + ast_use_defaults
> + } config_mode;
>  
>   enum ast_tx_chip tx_chip_type;
>   u8 dp501_maxclk;
> diff --git a/drivers/gpu/drm/ast/ast_main.c
> b/drivers/gpu/drm/ast/ast_main.c
> index 533e762..823c68f 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -62,13 +62,58 @@ uint8_t ast_get_index_reg_mask(struct ast_private
> *ast,
>   return ret;
>  }
>  
> +static void ast_detect_config_mode(struct drm_device *dev, u32
> *scu_rev)
> +{
> + struct device_node *np = dev->pdev->dev.of_node;
> + struct ast_private *ast = dev->dev_private;
> + uint32_t data, jreg;
> +
> + /* Check if we have device-tree properties */
> + if (np && !of_property_read_u32(np, "ast,scu-revision-id",
> scu_rev)) {
> + /* We do, disable P2A access */
> + ast->config_mode = ast_use_dt;
> + DRM_INFO("Using device-tree for configuration\n");
> + return;
> + }
> +
> + /*
> +  * The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
> +  * is disabled
> +  */
> + jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1,
> 0xff);
> + if (!(jreg & 0x10)) {
> + /* Double check it's actually working */
> + data = ast_read32(ast, 0xf004);
> + if (data != 0x) {
> + /* P2A works, grab silicon revision */
> + ast->config_mode = ast_use_p2a;
> +
> + DRM_INFO("Using P2A bridge for
> configuration\n");
> +
> + /* Read SCU7c (silicon revision register) */
> + ast_write32(ast, 0xf004, 0x1e6e);
> + ast_write32(ast, 0xf000

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Wed, 2016-11-23 at 10:03 +0200, Tomi Valkeinen wrote:
> Hi,
> 
> Since the fbdev framework is in maintenance mode and all new display drivers
> should be made with the DRM framework, remove the fbdev drivers from staging.
> 
> Note: the patches are created with git format-patch -D, so they can't be
> applied. Only for review.

I missed the discussion where this decision was made, I admit I am
unimpressed by it.

DRM drivers don't strike me as suitable for small/slow cores with dumb
framebuffers or simple 2D only accel, such as the one found in the ASpeed
BMCs.

With drmfb you basically have to shadow everything into memory & copy
over everything, and locks you out of simple 2D accel. For a simple text
console the result is orders of magnitude slower and memory hungry than
a simple fbdev.

At least that was the case last I looked at the DRM stuff with Dave,
maybe things have changed... 

Not everything has a powerful 3D GPU.

Ben.

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Thu, 2016-12-08 at 10:01 +0200, Tomi Valkeinen wrote:
> 
> > DRM drivers don't strike me as suitable for small/slow cores with dumb
> > framebuffers or simple 2D only accel, such as the one found in the ASpeed
> > BMCs.
> 
> Then the DRM framework should be improved to be suitable.

Dave ? :-)

> > With drmfb you basically have to shadow everything into memory & copy
> > over everything, and locks you out of simple 2D accel. For a simple text
> > console the result is orders of magnitude slower and memory hungry than
> > a simple fbdev.
> 
> I don't think that's true. You can have a single fbdev buffer and blit
> there all you want, afaik.

Well, I had that argument with Dave Airlie which I CCed. The "dumb" ones like
bochsdrmfb, cirrusdrmfb, astdrmfb ... all use shadowing, meaning they use a
lot more memory and cannot do any 2D acceleration for fbcon.

>From memory, David claimed you cannot directly work on the fb with a "proper"
DRM driver. Maybe I misunderstood but then the DRM shines by its complete
absence of useful documentation mixed with bazillion layers of APIs and helpers
so it's pretty hard to get ones head around it without wasting very large 
amounts
of time which I don't have at the moment.

> > Not everything has a powerful 3D GPU.
> 
> We don't use GPU on OMAPs (except for 3D).

The CPU in an OMAP is order of magnitude faster than what I have in an
Aspeed BMC though.

Ben.

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Thu, 2016-12-08 at 16:21 +0100, Daniel Vetter wrote:
> Yeah, small drivers like these we have piles now, things exploded a lot
> after atomic landed two years ago. And they seem to shrink with every
> release a bit more (since lots more drivers gives you lots more insight
> into what other refactorings would make sense). Those we have a big pile
> of, and nowadays (at least with developers expirienced with upstream, but
> not necessarily with drm) it takes but a few weeks from initial submission
> to getting them merged.
> 
> What we don't yet have a nice tidy example driver of is the even simpler
> "dumb framebuffer behind a slow bus with explicit/manual upload", for like
> small i2c/spi panels (and conceptually also usb, even though there bw and
> panel size are a bit scaled up). We've gained some really nice helpers for
> this this year, and there's 3 drivers in-flight to make use of it. But
> since that's right now just a hobbyist effort it's moving a bit slower
> (and I was mistaken a few weeks back where I assumed that one of them
> landed already).

What I find usually confusing is the interaction with the TTM and
overall fb memory management, when trying to plumb in simple 2d accel
to speed up fbcon mostly (but I don't mind making it available to user
space via ioctls, though that's not a priority).

As I mentioned earlier, probably 1 or 2 years ago, Dave made the
argument that shadowing through memory was necessary and precluded 2D
accel, though I don't fully remember the root of the argument. If that
is indeed not the case, then my main objection is lifted.

Cheers,
Ben.

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Fri, 2016-12-09 at 08:23 +1100, Benjamin Herrenschmidt wrote:
> > From memory, David claimed you cannot directly work on the fb with a 
> > "proper"
> 
> DRM driver. Maybe I misunderstood but then the DRM shines by its complete
> absence of useful documentation 

That sentence should have been in the past, it does look like
documentation has been landing in the tree this year ! yay ! I'll go
off read it.

> mixed with bazillion layers of APIs and helpers
> so it's pretty hard to get ones head around it without wasting very large 
> amounts
> of time which I don't have at the moment.

Ben.

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Fri, 2016-12-09 at 08:34 +1100, Benjamin Herrenschmidt wrote:
> As I mentioned earlier, probably 1 or 2 years ago, Dave made the
> argument that shadowing through memory was necessary and precluded 2D
> accel, though I don't fully remember the root of the argument. If that
> is indeed not the case, then my main objection is lifted.

Things seem to change quickly as Daniel pointed out.

So ast and cirrus seem to still use a manual dirty tracking and
shadowing (though I'm not sure why), but the infrastructure for
that has moved from the drivers to the helpers.

bochs (qemu) doesn't seem to anymore from what I can see as it
doesn't have a ->dirty callback.

Cheers,
Ben.

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Thu, 2016-12-08 at 11:10 +0100, Daniel Vetter wrote:
> > With drmfb you basically have to shadow everything into memory & copy
> > over everything, and locks you out of simple 2D accel. For a simple text
> > console the result is orders of magnitude slower and memory hungry than
> > a simple fbdev.
> 
> Not true, we have full fbdev emulation, and drivers can implement the 2d
> accel in there. And a bunch of them do. It's just that most teams decided
> that this is pointless waste of their time.j

Ok so my knowledge might be outdated here. I was complaining to Dave about
how cirrusdrmfb didn't even use blits for fbcon scrolling and always double
buffered everything, and Dave made the point that you basically had to do
that for security reasons that I mostly forgot the details of.

It looks like bochsdrmfb and astdrmfb are the same. If things have changed,
then cool. Can you point me to a drmfb driver that is a good (and not too
complex) example with simple 2d accel ? I'm thinking mostly of color
expansion, bitblt and solid fill for fbcon, the way I used to do it in
radeonfb for example.

> > At least that was the case last I looked at the DRM stuff with Dave,
> > maybe things have changed... 
> > 
> > Not everything has a powerful 3D GPU.
> 
> That's correct, and drm can cope. And compared to fbdev there's a very
> active community who improves it every kernel release to make it
> even better. Since about 2 years (when atomic landed) we merge new drivers at
> a rate of 2-3 per kernel release, and those new drivers get ever simpler
> and smaller thanks to all this work.

Yeah it's hard to follow from outside :-) As I said above, it would
help if you could point to a good modern example driver to use as
reference.

Thanks !

Cheers,
Ben.

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Fri, 2016-12-09 at 10:08 +1000, Dave Airlie wrote:
> What are people using fbcon for that needs acceleration, this is where I get
> a bit lost.
> 
> It's a console, if you aren't sshing into the machine.
> 
> It's main purpose should just be for gathering oopses and you've a lot better
> chance of getting an oops if you don't have some sketchy gpu accel in the way.

There are other uses for systems running Linux than being a server or desktop 
:-)

> The acceleration that most of the 2D things provide isn't ever that
> great, and shadowing is a lot more effective if done properly.

Not with a 400Mhz ARM9 processor on a fairly high res display. In these
case basic old things like color expansion for font rendering, bit
blits and solid fills for scrolls work beautifully. Anyway I just
realized that the ARM side of the AST GPU doesn't have the accel bits
at all anyway, only the host side, so I'm back to just a dumb FB. I
still want to avoid the copies though.

>  It's a feature
> that kernel ppl obsess over but I don't get a lot of real world feedback,
> (booting 9000 scsi nodes with debug on takes a long time was possibly
> something I heard once, and I think we resolved).

Ben.

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Fri, 2016-12-09 at 09:34 +0100, Daniel Vetter wrote:
> Yeah if you have discrete vram then your dumb display driver isn't all
> that pretty. We essentially just have the few drivers Dave hacked up to be
> able to boot some servers. And there's definitely lots of room for more
> shared code for those, and also some better infrastructure and helpers to
> share more cod and make them better.
> 
> The massive pile of dumb framebuffers we all merged over the past 2 years
> all use system/dma memory for scanout, and for those we have the very nice
> cma helpers that take care of everything for you. 

Do they work if the system/DMA memory has to be physically contiguous
and at a fixed address ? The AST "ARM side" GPU is like that.

> So it is possible, only reason vram dumb buffers look worse is that there's
> only 3 and no one cares about them, vs about 20 and a very active community
> of contributors (also for core drm improvements) for the other case.

Well, we could move offb to drm while at it I suppose that would be another
one (offb is the "dumb driver based on pre-programmed output by firmware).

> Althought the MXSFB driver that just landed does use ttm and vram, so
> maybe that's now improving too.

Cheers,
Ben.

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Fri, 2016-12-09 at 09:41 +0100, Daniel Vetter wrote:
> 
> And since I failed to make this clear: There's not really a
> fundamental
> reason ast and cirrus use the dirty tracking for fbdev. It's just that
> doing it that way was the fastest way to get those servers booting, and
> ever since no one cared. It's a bit tricky to do right because fbdev
> assumes it always own the framebuffer and that it never moves, 

That can be worked around from my memories of hacking fbdev many years
ago. Basically fbdev only owns it if it's the current VT and you can
make it release it if the user switches to KD_GRAPHICS which userspace
should always do before taking over.

As for multi userspace client, well, swapping an mmap between HW and
memory backing store is a somewhat solved problem already.

> whereas drm has a multi-master model and proper isolation. IIrc we've hacked 
> up
> something once, and if there's indeed more interest into vram dumb buffer
> drivers I'm pretty sure we can grow some nice ttm fb helpers (like the cma
> fb helpers we have) to make it all pretty and nice and fast and
> essentially plug-in-and-forget from a driver authors pov.

That would be nice. I don't have the bandwidth to swap-in enough
understanding of TTM guts right now but I might look into it some time next 
year if nobody beats me to it.

> Cheers, Daniel
> 
> > The massive pile of dumb framebuffers we all merged over the past 2 years
> > all use system/dma memory for scanout, and for those we have the very nice
> > cma helpers that take care of everything for you. So it is possible, only
> > reason vram dumb buffers look worse is that there's only 3 and no one
> > cares about them, vs about 20 and a very active community of contributors
> > (also for core drm improvements) for the other case.
> > 
> > Althought the MXSFB driver that just landed does use ttm and vram, so
> > maybe that's now improving too.
> > -Daniel
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> 

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Fri, 2016-12-09 at 14:57 +0100, David Herrmann wrote:
> Despite all of this I still see no reason why a driver could not
> expose the static, real frambuffers via private ioctls. You can get
> all your fancy acceleration that way. Then fix user-space to use this
> API. If enough drivers end up with something similar, move it into the
> core. Just like we always do in DRM.

I don't care so much about userspace in my specific use case, more
about fbcon, which I think can be solved without too many hoops.

As for FB objects, my thinking is we could just use
unmap_mapping_ranges() to effectively change the mapping under the hood
of the app so it alternatively maps a bit of fb or a bit of memory...

Cheers,
Ben.

[RFC PATCH 0/3] staging: remove fbdev drivers

[Benjamin Herrenschmidt]

On Fri, 2016-12-09 at 14:35 +0100, Daniel Vetter wrote:
> > As for multi userspace client, well, swapping an mmap between HW and
> > memory backing store is a somewhat solved problem already.
> 
> Hm, I didn't know that, but then all existing drm drivers have fairly
> simplistic fbdev mmap implementations.

Hrm, I though the TTM did it ... I remember talking with Thomas
Hellstrom about that back in the day... you use unmap_mapping_range
to unmap the existing mappings basically so you can take new faults
and route them to a different page, but I can't see a call in there
so maybe he ended up not doing it.

We used to do that on Cell to "context switch" the local memory of
the SPU engines between the real SPU and the backing store. It's not
very hard to do.

The main issue is that the mapping attributes change between cached
and non-cached under the hood, so users have to be careful not to do
things like use instructions that only work on one type of mapping
(or do things like misaligned accesses).

> > > whereas drm has a multi-master model and proper isolation. IIrc we've 
> > > hacked up
> > > something once, and if there's indeed more interest into vram dumb buffer
> > > drivers I'm pretty sure we can grow some nice ttm fb helpers (like the cma
> > > fb helpers we have) to make it all pretty and nice and fast and
> > > essentially plug-in-and-forget from a driver authors pov.
> > 
> > That would be nice. I don't have the bandwidth to swap-in enough
> > understanding of TTM guts right now but I might look into it some time 
> > next 
> > year if nobody beats me to it.
> 
> Probably best would be to first extract some helpers for ttm based vram
> dumb buffer management, and then start to implement some of the
> improvements so that all drivers can benefit. Like you've said it's not
> rocket science, it just needs to be done ;-)

Right :-)

Though getting ones head around the infrastructure in the DRM does take
time :-) There's a lot of stuff in there, between TTM, GEM etc... and
not all of it completely "obvious" ...

Cheers,
Ben.

> -Daniel
> -- 

[PATCH] drm: Fix broken use of _PAGE_NO_CACHE on powerpc

[Benjamin Herrenschmidt]

That constant isn't meant to be used outside of arch mm code

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/gpu/drm/drm_memory.c  | 2 +-
 drivers/gpu/drm/drm_scatter.c | 2 +-
 drivers/gpu/drm/drm_vm.c  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
index 87a8cb7..fc0ebd2 100644
--- a/drivers/gpu/drm/drm_memory.c
+++ b/drivers/gpu/drm/drm_memory.c
@@ -44,7 +44,7 @@
 # include 
 #else
 # ifdef __powerpc__
-#  define PAGE_AGP __pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)
+#  define PAGE_AGP pgprot_noncached_wc(PAGE_KERNEL)
 # else
 #  define PAGE_AGP PAGE_KERNEL
 # endif
diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c
index 4f0f3b3..bf70431 100644
--- a/drivers/gpu/drm/drm_scatter.c
+++ b/drivers/gpu/drm/drm_scatter.c
@@ -41,7 +41,7 @@
 static inline void *drm_vmalloc_dma(unsigned long size)
 {
 #if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE)
-   return __vmalloc(size, GFP_KERNEL, PAGE_KERNEL | _PAGE_NO_CACHE);
+   return __vmalloc(size, GFP_KERNEL, pgprot_noncached_wc(PAGE_KERNEL));
 #else
return vmalloc_32(size);
 #endif
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index ac9f4b3..7e9f642 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -80,7 +80,7 @@ static pgprot_t drm_dma_prot(uint32_t map_type, struct 
vm_area_struct *vma)
pgprot_t tmp = vm_get_page_prot(vma->vm_flags);

 #if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE)
-   tmp |= _PAGE_NO_CACHE;
+   tmp = pgprot_noncached_wc(tmp);
 #endif
return tmp;
 }
@@ -593,7 +593,7 @@ static int drm_mmap_locked(struct file *filp, struct 
vm_area_struct *vma)
 * pages and mappings in fault()
 */
 #if defined(__powerpc__)
-   pgprot_val(vma->vm_page_prot) |= _PAGE_NO_CACHE;
+   vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
 #endif
vma->vm_ops = _vm_ops;
break;

Stupid NVIDIA 3D vgaarb.c patch

[Benjamin Herrenschmidt]

On Mon, 2014-09-22 at 13:43 -0700, Linus Torvalds wrote:
> Adding proper people and mailing lists..
> 
> The PCI_CLASS_DISPLAY_VGA test goes back to the very beginning by
> BenH, and I have no idea if adding PCI_CLASS_DISPLAY_3D is
> appropriate, but hopefully somebody does. The fact that it makes
> things work certainly argues fairly convincingly for it, but I want
> some backup here.
> 
> Dave, BenH?
> 
> Christopher(?), can you please also specify which laptop etc. And the
> patch itself seems to have come from somebody else, unless you're
> Lekensteyn. So we'd want to get the provenance of that too.

Hrm, that sucks. "3D" could mean anything really, we might need an
explicit vendor ID check as well and maybe even device ID ...

Ben.

> Linus
> 
> On Mon, Sep 22, 2014 at 1:28 PM, C Bergstr?m  
> wrote:
> > Hi Linus,
> >
> > I don't know who the original author is and I can have one of the distro
> > graphics friends review it, but I need this patch below to get NVIDIA
> > drivers to work (without using any Intel graphics) on my laptop
> > http://pastebin.com/wpmFi38k
> >
> > Sorry - I know this is not the proper way to submit a patch, but it's
> > trivial and I'm not a kernel dev.
> >
> > Thanks
> >
> > ./C