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

2017-02-20 Thread Emil Velikov 
On 18 February 2017 at 22:22, Benjamin Herrenschmidt
 wrote:
> 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 stand corrected - yes the original code did not have the
[ddr_test_2500 always returns false] bug.
Pardon, for my earlier blunder.

Regards,
Emil
___
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

2017-02-18 Thread 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 2/2] drm/ast: Support AST2500

2017-02-18 Thread Emil Velikov 
On 18 February 2017 at 03:41, Benjamin Herrenschmidt
 wrote:
> 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.
>
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.

> 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 2/2] drm/ast: Support AST2500

2017-02-18 Thread Emil Velikov 
On 17 February 2017 at 21:36, Benjamin Herrenschmidt
 wrote:
> 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.
>
Heh, I tried to be subtle here and not point fingers, but seemingly
that came out confusing ;-)

Afaict the code is 'broken' since it was sent to the ML by Y.C. Chen,
thus they likely [seemingly] haven't tested the POST either.
It's not my call how (or which parts) it should go it. Merely pointing
out what seems like bugs/issues.

Regards,
Emil
___
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

2017-02-17 Thread 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 2/2] drm/ast: Support AST2500

2017-02-17 Thread 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 2/2] drm/ast: Support AST2500

2017-02-17 Thread Emil Velikov 
Hi Ben,

On 16 February 2017 at 21:09, Benjamin Herrenschmidt
 wrote:
> On Thu, 2017-02-16 at 17:33 +, Emil Velikov wrote:
>> On 16 February 2017 at 09:09, Benjamin Herrenschmidt
>>  wrote:
>> > From: "Y.C. Chen" 
>> >
>> > 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 
>> > Signed-off-by: Benjamin Herrenschmidt 
>> > ---
>> > 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.
>
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:

>> > +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 ?
>>
This here.

Regards,
Emil
___
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

2017-02-16 Thread 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

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

2017-02-16 Thread 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

2017-02-16 Thread 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

2017-02-16 Thread 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

2017-02-16 Thread Benjamin Herrenschmidt 
On Thu, 2017-02-16 at 17:33 +, Emil Velikov wrote:
> On 16 February 2017 at 09:09, Benjamin Herrenschmidt
>  wrote:
> > From: "Y.C. Chen" 
> > 
> > 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 
> > Signed-off-by: Benjamin Herrenschmidt 
> > ---
> > 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 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;
> >  

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

2017-02-16 Thread Emil Velikov 
On 16 February 2017 at 09:09, Benjamin Herrenschmidt
 wrote:
> From: "Y.C. Chen" 
>
> 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 
> Signed-off-by: Benjamin Herrenschmidt 
> ---
> 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 ;-)


> +/*
> + * 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 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,

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

2017-02-16 Thread Benjamin Herrenschmidt 
On Thu, 2017-02-16 at 20:09 +1100, Benjamin Herrenschmidt wrote:
> From: "Y.C. Chen" 
> 
> 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 
> Signed-off-by: Benjamin Herrenschmidt 
> ---
> 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