drm/amd/display: add SI support to AMD DC

[PATCH 01/10] drm/amd/display: add asics info for SI parts [PATCH 02/10] drm/amd/display: dc/dce: add DCE6 support [PATCH 03/10] drm/amd/display: dc/core: add DCE6 support [PATCH 04/10] drm/amd/display: dc/bios: add support for DCE6 [PATCH 05/10] drm/amd/display: dc/gpio: add support for DCE6 [PATCH 06/10] drm/amd/display: dc/i2caux: add support for DCE6 [PATCH 07/10] drm/amd/display: dc/irq: add support for DCE6 [PATCH 08/10] drm/amd/display: amdgpu_dm: add SI support [PATCH 09/10] drm/amdgpu: enable DC support for SI parts [PATCH 10/10] drm/amd/display: enable SI support in the Kconfig

The series adds preliminar SI support as a Proof Of Concept, based on the idea that DCE6 is similar to DCE8, to be reviewed and refined

Android-x86 need/motivation lies in the following chain of dependencies:
Vulkan radv requires gbm gralloc prime_fd support,
gbm gralloc requires drm hwcomposer,
drm hwcomposer requires Atomic Display Framework,
Atomic Display Framework requires AMD DC, currently not supporting SI.

So the goals are:
1) to get Vulkan radv working on SI parts for android-x86.
2) to remove the gap in SI (GCN 1st gen) not having atomic support.

DCE6 specific code was implemented as a replica of existing DCE8 support and based on how DCE8 specific code was added on top of DCE10,11 support by adding dce60* sources, functions, macros for each existing in dce80*

CONFIG_DRM_AMD_DC_SI parameter has been added to control SI support in DC

During this first iteration of review, there are aspects to verify:
- dce60 code has been added mechanically, so there may be redundancies and space for refactoring part of the code
- dce60_resources was having too many building errors due to missing DCE6 macros
in order to temporarily overcome the problem dce_8_0_{d,sh_mask}.h headers were used for the PoC
- dc/irq suffered the same problem dce_8_0_{d,sh_mask}.h headers were used for the PoC
- gfx6 may require some ad hoc initialization, skipped for the moment
- Hainan specific code requires review, as some documentation and code paths
seem to point that famility may not have DCE6, please confirm
- video decoding blocks code have not been touched
- dc/dce/dce_clock_source.{c,h} may be missing some SI/DCE6 specifics
- dc/dce/dce_dmcu.{c,h} may be missing some SI/DCE6 specifics
- dc/dce/dce_hwseq.h may be missing some SI/DCE6 specifics
- dc/dce/dce_link_encoder.h may be missing some SI/DCE6 specifics
- dc/dce/dce_stream_encoder.h may be missing some SI/DCE6 specifics
- dc/amdgpu_dm/* changes may be incomplete
- Any other omissis to be reviewed
- Feedback on best testing strategy required

Review from an expert of the DC impacted modules is recommended

SW Layer
/===============================================================\

| DC        Display     Timing          Mode        Asic        |
| Interface Service     Service         Manager     Capability* |
|                                                               |
| Display   Topology    Display         Link        Adapter     |
| Path      Manager     Capability      Service     Service     |
|                       Service                                 |
|---------------------------------------------------------------|
| GPIO*     IRQ         I2cAux          HW          BIOS        |
|           Service**   Manager*        Sequencer*  Parser*     |
|                                                               |
| Connector Encoder     Audio           GPU         Controller  |
|                                                               |

\===============================================================/
HW Layer

Legend:
*dce60 support was added cleanly with dce_6_0_{d,sh_mask}.h headers **dce60 support was added using dce_8_0_{d,sh_mask}.h headers

Android-x86 preliminary tests results:

[Boots with drm gralloc]
3DMark Slingshot
GFXbench OpenGLES benchmarks OK
V1 GPU benchmark (OpenGLES) OK
Regression in Google Chrome, Youtube (app does not show up) Regression in Olympus Rising, Chicken Invaders (app does not show up)

[Boots with drm hwcomposer + gbm gralloc]
Google Chrome, Youtube are OK
Vulkan radv HAL API becomes available with hwc+gbm gralloc V1 GPU benchmark (Vulkan API) OK
Sacha Willems examples OK
Some glitch/freeze in 3DMark Slingshot Extreeme and API overhead

Kind regards

Mauro Rossi
android-x86 team

_______________________________________________
amd-gfx mailing list [email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Hi Mauro

Do you know if there are any plans to add in UVD support on SI too?

Thanks

Mike

Quoted 95 lines

[PATCH 01/10] drm/amd/display: add asics info for SI parts [PATCH 02/10] drm/amd/display: dc/dce: add DCE6 support [PATCH 03/10] drm/amd/display: dc/core: add DCE6 support [PATCH 04/10] drm/amd/display: dc/bios: add support for DCE6 [PATCH 05/10] drm/amd/display: dc/gpio: add support for DCE6 [PATCH 06/10] drm/amd/display: dc/i2caux: add support for DCE6 [PATCH 07/10] drm/amd/display: dc/irq: add support for DCE6 [PATCH 08/10] drm/amd/display: amdgpu_dm: add SI support [PATCH 09/10] drm/amdgpu: enable DC support for SI parts [PATCH 10/10] drm/amd/display: enable SI support in the Kconfig

The series adds preliminar SI support as a Proof Of Concept, based on the idea that DCE6 is similar to DCE8, to be reviewed and refined

Android-x86 need/motivation lies in the following chain of dependencies:
Vulkan radv requires gbm gralloc prime_fd support,
gbm gralloc requires drm hwcomposer,
drm hwcomposer requires Atomic Display Framework,
Atomic Display Framework requires AMD DC, currently not supporting SI.

So the goals are:
1) to get Vulkan radv working on SI parts for android-x86.
2) to remove the gap in SI (GCN 1st gen) not having atomic support.

DCE6 specific code was implemented as a replica of existing DCE8 support and based on how DCE8 specific code was added on top of DCE10,11 support by adding dce60* sources, functions, macros for each existing in dce80*

CONFIG_DRM_AMD_DC_SI parameter has been added to control SI support in DC

During this first iteration of review, there are aspects to verify:
- dce60 code has been added mechanically, so there may be redundancies and space for refactoring part of the code
- dce60_resources was having too many building errors due to missing DCE6 macros
in order to temporarily overcome the problem dce_8_0_{d,sh_mask}.h headers were used for the PoC
- dc/irq suffered the same problem dce_8_0_{d,sh_mask}.h headers were used for the PoC
- gfx6 may require some ad hoc initialization, skipped for the moment
- Hainan specific code requires review, as some documentation and code paths
seem to point that famility may not have DCE6, please confirm
- video decoding blocks code have not been touched
- dc/dce/dce_clock_source.{c,h} may be missing some SI/DCE6 specifics
- dc/dce/dce_dmcu.{c,h} may be missing some SI/DCE6 specifics
- dc/dce/dce_hwseq.h may be missing some SI/DCE6 specifics
- dc/dce/dce_link_encoder.h may be missing some SI/DCE6 specifics
- dc/dce/dce_stream_encoder.h may be missing some SI/DCE6 specifics
- dc/amdgpu_dm/* changes may be incomplete
- Any other omissis to be reviewed
- Feedback on best testing strategy required

Review from an expert of the DC impacted modules is recommended

SW Layer
/===============================================================\

| DC        Display     Timing          Mode        Asic        |
| Interface Service     Service         Manager     Capability* |
|                                                               |
| Display   Topology    Display         Link        Adapter     |
| Path      Manager     Capability      Service     Service     |
|                       Service                                 |
|---------------------------------------------------------------|
| GPIO*     IRQ         I2cAux          HW          BIOS        |
|           Service**   Manager*        Sequencer*  Parser*     |
|                                                               |
| Connector Encoder     Audio           GPU         Controller  |
|                                                               |

\===============================================================/
HW Layer

Legend:
*dce60 support was added cleanly with dce_6_0_{d,sh_mask}.h headers **dce60 support was added using dce_8_0_{d,sh_mask}.h headers

Android-x86 preliminary tests results:

[Boots with drm gralloc]
3DMark Slingshot
GFXbench OpenGLES benchmarks OK
V1 GPU benchmark (OpenGLES) OK
Regression in Google Chrome, Youtube (app does not show up) Regression in Olympus Rising, Chicken Invaders (app does not show up)

[Boots with drm hwcomposer + gbm gralloc]
Google Chrome, Youtube are OK
Vulkan radv HAL API becomes available with hwc+gbm gralloc V1 GPU benchmark (Vulkan API) OK
Sacha Willems examples OK
Some glitch/freeze in 3DMark Slingshot Extreeme and API overhead

Kind regards

Quoted 8 lines

Mauro Rossi
android-x86 team

_______________________________________________
amd-gfx mailing list [email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Hi Mike,
On Mon, Oct 8, 2018 at 1:00 PM Mike Lothian <[email protected] <mailto:[email protected]>> wrote:

Hi Mauro

Do you know if there are any plans to add in UVD support on SI too?

Thanks

Mike

At the moment my focus is on getting a conformant, working and stable implementation of Atomic Display Framework, with the objective to have it upstreamed to amd-gfx branch, then staging (drm-next) and maybe merged in linux kernel.

To be honest my attempt is based on code paths inspection and mimicking,
so in this moment I do not even know the state of UVD and what changes are needed, but, based on what I saw for DCE6 support addition on top of DCE8,
covering all compatible HW modules makes a lot of sense and it is an opportunity to exploit, if feasible.

For this to happen in most complete and reliable way the feedback of staff who worked on DAL/DC will be essential, because what I did now was to adapt code for DCE8 to work for DCE6,
but it was like an "optimistic monkey with a keyboard" approach, with all due respect for monkeys with keyboards, :-) I may have missed dozen of changes.

Mauro

On Mon, 8 Oct 2018 at 03:24 Mauro Rossi <[email protected] <mailto:[email protected]>> wrote:

[PATCH 01/10] drm/amd/display: add asics info for SI parts [PATCH 02/10] drm/amd/display: dc/dce: add DCE6 support [PATCH 03/10] drm/amd/display: dc/core: add DCE6 support [PATCH 04/10] drm/amd/display: dc/bios: add support for DCE6 [PATCH 05/10] drm/amd/display: dc/gpio: add support for DCE6 [PATCH 06/10] drm/amd/display: dc/i2caux: add support for DCE6 [PATCH 07/10] drm/amd/display: dc/irq: add support for DCE6 [PATCH 08/10] drm/amd/display: amdgpu_dm: add SI support [PATCH 09/10] drm/amdgpu: enable DC support for SI parts [PATCH 10/10] drm/amd/display: enable SI support in the Kconfig

The series adds preliminar SI support as a Proof Of Concept, based on the idea that DCE6 is similar to DCE8, to be reviewed and refined

Android-x86 need/motivation lies in the following chain of dependencies:
Vulkan radv requires gbm gralloc prime_fd support, gbm gralloc requires drm hwcomposer,
drm hwcomposer requires Atomic Display Framework, Atomic Display Framework requires AMD DC, currently not supporting SI.

So the goals are:
1) to get Vulkan radv working on SI parts for android-x86.
2) to remove the gap in SI (GCN 1st gen) not having atomic support.

DCE6 specific code was implemented as a replica of existing DCE8 support
and based on how DCE8 specific code was added on top of DCE10,11 support
by adding dce60* sources, functions, macros for each existing in dce80*

CONFIG_DRM_AMD_DC_SI parameter has been added to control SI support in DC

During this first iteration of review, there are aspects to verify:
- dce60 code has been added mechanically, so there may be redundancies
and space for refactoring part of the code
- dce60_resources was having too many building errors due to missing DCE6 macros
in order to temporarily overcome the problem dce_8_0_{d,sh_mask}.h headers
were used for the PoC
- dc/irq suffered the same problem dce_8_0_{d,sh_mask}.h headers were used for the PoC
- gfx6 may require some ad hoc initialization, skipped for the moment
- Hainan specific code requires review, as some documentation and code paths
seem to point that famility may not have DCE6, please confirm
- video decoding blocks code have not been touched
- dc/dce/dce_clock_source.{c,h} may be missing some SI/DCE6 specifics
- dc/dce/dce_dmcu.{c,h} may be missing some SI/DCE6 specifics
- dc/dce/dce_hwseq.h may be missing some SI/DCE6 specifics
- dc/dce/dce_link_encoder.h may be missing some SI/DCE6 specifics
- dc/dce/dce_stream_encoder.h may be missing some SI/DCE6 specifics
- dc/amdgpu_dm/* changes may be incomplete
- Any other omissis to be reviewed
- Feedback on best testing strategy required

Review from an expert of the DC impacted modules is recommended

    SW Layer /===============================================================\
| DC        Display     Timing          Mode Asic        | | Interface Service     Service         Manager  Capability* | |        |
| Display   Topology    Display         Link Adapter     | | Path      Manager     Capability      Service  Service     | |                       Service        | |---------------------------------------------------------------| | GPIO*     IRQ         I2cAux          HW BIOS        | |           Service**   Manager*        Sequencer* Parser*     | |        |
| Connector Encoder     Audio           GPU  Controller  | |        | \===============================================================/ HW Layer

Legend:
*dce60 support was added cleanly with dce_6_0_{d,sh_mask}.h headers
**dce60 support was added using dce_8_0_{d,sh_mask}.h headers

Android-x86 preliminary tests results:

[Boots with drm gralloc]
3DMark Slingshot
GFXbench OpenGLES benchmarks OK
V1 GPU benchmark (OpenGLES) OK
Regression in Google Chrome, Youtube (app does not show up) Regression in Olympus Rising,  Chicken Invaders (app does not show up)

[Boots with drm hwcomposer + gbm gralloc]
Google Chrome, Youtube are OK
Vulkan radv HAL API becomes available with hwc+gbm gralloc V1 GPU benchmark (Vulkan API) OK
Sacha Willems examples OK
Some glitch/freeze in 3DMark Slingshot Extreeme and API overhead

Kind regards

Mauro Rossi
android-x86 team

_______________________________________________
amd-gfx mailing list [email protected] <mailto:[email protected]>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list [email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

UVD/VCE on SI with amdgpu would need new firmware.

And so far we never had time to actually look into releasing that firmware.

Regards,
Christian.

Am 08.10.2018 um 13:22 schrieb Mauro Rossi:

Hi Mike,

Quoted 8 lines · Mike Lothian

Hi Mauro

Do you know if there are any plans to add in UVD support on SI too?

Thanks

Mike

At the moment my focus is on getting a conformant, working and stable implementation of Atomic Display Framework, with the objective to have it upstreamed to amd-gfx branch, then staging (drm-next) and maybe merged in linux kernel.

To be honest my attempt is based on code paths inspection and mimicking, so in this moment I do not even know the state of UVD and what changes are needed,
but, based on what I saw for DCE6 support addition on top of DCE8, covering all compatible HW modules makes a lot of sense and it is an opportunity to exploit,
if feasible.

For this to happen in most complete and reliable way the feedback of staff who worked on DAL/DC
will be essential, because what I did now was to adapt code for DCE8 to work for DCE6,
but it was like an "optimistic monkey with a keyboard" approach, with all due respect for monkeys with keyboards,
:-) I may have missed dozen of changes.

Mauro

Quoted 99 lines · On Mon, 8 Oct 2018 at 03:24 Mauro Rossi <issor.oruam-Re5JQEeQqe8AvxtiuMwx3w@pub…

Quoted 97 lines

[PATCH 01/10] drm/amd/display: add asics info for SI parts [PATCH 02/10] drm/amd/display: dc/dce: add DCE6 support [PATCH 03/10] drm/amd/display: dc/core: add DCE6 support [PATCH 04/10] drm/amd/display: dc/bios: add support for DCE6 [PATCH 05/10] drm/amd/display: dc/gpio: add support for DCE6 [PATCH 06/10] drm/amd/display: dc/i2caux: add support for DCE6 [PATCH 07/10] drm/amd/display: dc/irq: add support for DCE6 [PATCH 08/10] drm/amd/display: amdgpu_dm: add SI support [PATCH 09/10] drm/amdgpu: enable DC support for SI parts [PATCH 10/10] drm/amd/display: enable SI support in the Kconfig

The series adds preliminar SI support as a Proof Of Concept, based on the idea that DCE6 is similar to DCE8, to be reviewed and refined

Android-x86 need/motivation lies in the following chain of dependencies:
Vulkan radv requires gbm gralloc prime_fd support,
gbm gralloc requires drm hwcomposer,
drm hwcomposer requires Atomic Display Framework,
Atomic Display Framework requires AMD DC, currently not supporting SI.

So the goals are:
1) to get Vulkan radv working on SI parts for android-x86.
2) to remove the gap in SI (GCN 1st gen) not having atomic support.

DCE6 specific code was implemented as a replica of existing DCE8 support and based on how DCE8 specific code was added on top of DCE10,11 support by adding dce60* sources, functions, macros for each existing in dce80*

CONFIG_DRM_AMD_DC_SI parameter has been added to control SI support in DC

During this first iteration of review, there are aspects to verify:
- dce60 code has been added mechanically, so there may be redundancies and space for refactoring part of the code
- dce60_resources was having too many building errors due to missing DCE6 macros
in order to temporarily overcome the problem dce_8_0_{d,sh_mask}.h headers
were used for the PoC
- dc/irq suffered the same problem dce_8_0_{d,sh_mask}.h headers were used for the PoC
- gfx6 may require some ad hoc initialization, skipped for the moment
- Hainan specific code requires review, as some documentation and code paths
seem to point that famility may not have DCE6, please confirm
- video decoding blocks code have not been touched
- dc/dce/dce_clock_source.{c,h} may be missing some SI/DCE6 specifics
- dc/dce/dce_dmcu.{c,h} may be missing some SI/DCE6 specifics
- dc/dce/dce_hwseq.h may be missing some SI/DCE6 specifics
- dc/dce/dce_link_encoder.h may be missing some SI/DCE6 specifics
- dc/dce/dce_stream_encoder.h may be missing some SI/DCE6 specifics
- dc/amdgpu_dm/* changes may be incomplete
- Any other omissis to be reviewed
- Feedback on best testing strategy required

Review from an expert of the DC impacted modules is recommended

SW Layer
/===============================================================\

| DC        Display     Timing          Mode        Asic        |
| Interface Service     Service         Manager     Capability* |
|                                                               |
| Display   Topology    Display         Link        Adapter     |
| Path      Manager     Capability      Service     Service     |
|                       Service                                 |
|---------------------------------------------------------------|
| GPIO*     IRQ         I2cAux          HW          BIOS        |
|           Service**   Manager*        Sequencer*  Parser*     |
|                                                               |
| Connector Encoder     Audio           GPU         Controller  |
|                                                               |

\===============================================================/
HW Layer

Legend:
*dce60 support was added cleanly with dce_6_0_{d,sh_mask}.h headers **dce60 support was added using dce_8_0_{d,sh_mask}.h headers

Android-x86 preliminary tests results:

[Boots with drm gralloc]
3DMark Slingshot
GFXbench OpenGLES benchmarks OK
V1 GPU benchmark (OpenGLES) OK
Regression in Google Chrome, Youtube (app does not show up) Regression in Olympus Rising, Chicken Invaders (app does not show up)

[Boots with drm hwcomposer + gbm gralloc]
Google Chrome, Youtube are OK
Vulkan radv HAL API becomes available with hwc+gbm gralloc V1 GPU benchmark (Vulkan API) OK
Sacha Willems examples OK
Some glitch/freeze in 3DMark Slingshot Extreeme and API overhead

Kind regards

Quoted 9 lines

Quoted 8 lines

Mauro Rossi
android-x86 team

_______________________________________________
amd-gfx mailing list [email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing listamd-gfx-PD4FTy7X32lNgt0PjOBp934avgP/[email protected]://lists.freedesktop.org/mailman/listinfo/amd-gfx

The series adds preliminar SI support as a Proof Of Concept, based on the idea that DCE6 is similar to DCE8, to be reviewed and refined

Android-x86 need/motivation lies in the following chain of dependencies:
Vulkan radv requires gbm gralloc prime_fd support,
gbm gralloc requires drm hwcomposer,
drm hwcomposer requires Atomic Display Framework,
Atomic Display Framework requires AMD DC, currently not supporting SI.

So the goals are:
1) to get Vulkan radv working on SI parts for android-x86.

2) to remove the gap in SI (GCN 1st gen) not having atomic support.

I am currently testing your patch set, on amd-staging-drm-next (380d480842d584278dba9aa74341017d8c7d8c23) with an AMD tahiti xt part and a displayport monitor.

I am currently testing your patch set, on amd-staging-drm-next (380d480842d584278dba9aa74341017d8c7d8c23) with an AMD tahiti xt part and a displayport monitor.
patch02 drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c did not apply but seems kind of benign.

It's working out of the box on my AMD tahiti xt part. I did not manage to break it with aggressive mode programming. Let's see how it goes with my everday usage.

Quoted 3 lines

The series adds preliminar SI support as a Proof Of Concept, based on the idea that DCE6 is similar to DCE8, to be reviewed and refined

Did want to do it, but did drop it due to DC code getting fixed with too much changes.
Brutally mapping DCE6 to DCE8 is an act of faith... and it's working on my part.

Quoted 8 lines

Android-x86 need/motivation lies in the following chain of dependencies:
Vulkan radv requires gbm gralloc prime_fd support,
gbm gralloc requires drm hwcomposer,
drm hwcomposer requires Atomic Display Framework,
Atomic Display Framework requires AMD DC, currently not supporting SI.

Quoted 2 lines

So the goals are:
1) to get Vulkan radv working on SI parts for android-x86.

AFAIK, Vulkan support is not dependent on the display block. I am running heavy vulkan games on a custom gnu/linux x86_64 AMD hardware based system, then the hwcomposer is android only?

Quoted 2 lines

2) to remove the gap in SI (GCN 1st gen) not having atomic support.

I was nearly sure that atomic support was implicitely added for parts supporting only legacy DRM mode programming interfaces?

--
Sylvain

Thanks for info, do you have some github or patch to share for comparison/mutual knowledge?

Sorry, I was wrong: I thought your patch set was enabling by default DC for dce6 (it requires the DC kernel param).
I did force it, and it fails to init the dce6.

Sylvain - I did hack a bit your patch set on amd-staging-drm-next to make it go through the asic init and I managed to get a x11 display with lines kind of garbled, but you can still understand easily what's on the screen.

Sylvain - ... The lines may be garbled in your driver code because, if I recall properly, "line buffer" programing in dce8 is not
the same than in dce6 (look for registers with the "LB" abbreviation). Or some slight differences in frame buffer tiling.

Sylvain - I checked the kernel log, and like you said, I got errors in DM_PPLIB due to an invalid powerlevel and atombios/vbios table parsing regarding connectors. general dpm is in amdgpu(no DC) for SI, it means the DCE related dpm part in current SI amdgpu code path should be "copied" in DC. It is related very probably to the parsing of VBIOS/ATOMBIOS tables.

10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!
10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!

Sylvain - Did add SI handling in some raven firmware loader function. In drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c, "load_dmcu_fw" function augmented with SI chip asic_type.

Sylvain - AFAIK, the real thing that you additionally get with DC is freesync. But freesync is actually going to be interesting only if displays are able to get their sync range lower bound to 0, and get significant power saving thanks to this. For the use case of very low display refresh rate I don't even think displayport or hdmi can do that, and be power friendly (you would have to retrain the link probably each time you send a framebuffer to the display).

Mauro -- dce60_resources was having too many building errors due to missing DCE6 macros in order to temporarily overcome the problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

Mauro - dc/irq suffered the same problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

Mauro - gfx6 may require some ad hoc initialization, skipped for the moment

Mauro - Hainan specific code requires review, as some documentation and code paths seem to point that famility may not have DCE6, please confirm

Mauro - video decoding blocks code have not been touched

Mauro - dc/dce/dce_clock_source.{c,h} may be missing some SI/DCE6 specifics

Sylvain - It _seems_ there is not that much additional work to do in order to make it properly work.

Testing on x11,wayland or other ways

DOUBT: I think that it would make sense to set "power level 0" i.e. the "lower state" as safe default,
considering that powerplay smu6/hwmgr are not implemented for SI and smu7 CIK functions do not work,
the AS-IS dpm is the only available option. (and it seams to be working, looking at the framerates 250-280 in the V1 Vulkan benchmark)

If freesync is about reducing the framerate rate for power saving, provided that I've seen it be mentioned the first time for GCN 2nd generation, I'm not expecting freesync as a mandatory capability for the series.

In amd-staging-drm-next dce_clock_source is generic, SI specifics are not necessary anymore.

Any other testing tools worth a run?

Quoted 6 lines

DOUBT: I think that it would make sense to set "power level 0" i.e. the "lower state" as safe default,
considering that powerplay smu6/hwmgr are not implemented for SI and smu7 CIK functions do not work,
the AS-IS dpm is the only available option. (and it seams to be working, looking at the framerates 250-280 in the V1 Vulkan benchmark)

dpm not implemented for SI? Are you sure? Because I recall passing "messages"

switch back and forth between "high" and "auto". And if I stay on "high" my GPU
fans are going crazy all the time, which forces me to switch back to "auto" in
order to quiet the fans.
Any major state change for any block would have to go through the dpm block (smu), because it has to be accounted by this block for proper operation. One
of the hard thing to code, coze no proper documentation, was to set the blocks
in an initial state from a poweron/hard reset/etc which the dpm will know how to start/catch-up its operations from. In a perfect world, the dpm block
would have the knowledge on how to program any block in an initial working state from any unknown state (and in case of hard block hang, would know how to
hard reset it and set it in a initial state).

Quoted 2 lines

If freesync is about reducing the framerate rate for power saving, provided that I've seen it be mentioned the first time for GCN 2nd

generation,

Quoted 1 line

I'm not expecting freesync as a mandatory capability for the series.

Well, there are the low FPS games, and the movies. I am a regular gamer, and I
know that sustained 60 fps is the very lower bound for many games on a desktop
computer display.
It does not apply to mobile size display, and "couch playing" on a TV. The readibilty of the game depends directly on the amplitude of movements in physical distance on the display, their speed, the viewing distance, and the
FPS. From my personal comfort point of view and types of games I play, on a desktop display (dpi, typical viewing distance, typical size), I would target a
minimum of 80-100 fps (120/144 fps seems to be "perfect" targets). Played rise of the tomb raider (vulkan) on my tahiti part, and even with a blur
effect to "smooth" the perceived fps like in movies, sub-60 fps is really uncomfortable. I was about to stop playing to this game even though it is a rather "slow" video game.

Quoted 2 lines

In amd-staging-drm-next dce_clock_source is generic, SI specifics are not necessary anymore.

If I recall properly, "bandwidths" and "watermarks" calcs had asic generations
specific code paths. bandwidth is important for displayport programming or maybe you can presume the maximum all the time (and sacrifice some unkown amount of power) and watermarks, I know in my old driver I was giving the highest priority the the dce block anyway (something related to a "memory aribtrer" and "line buffer").

Quoted 1 line

Any other testing tools worth a run?

AAA games (vulkan: rize of the tomb raider, GL:bioshock infinite) and AAAA games
(dota2(gl and vulkan), cs:go...)

--
Sylvain

dpm for SI is available, while powerplay for SI is not, but display/amdgpu_dm uses some powerplay calls, where get_static_clock functions not available and the *ERROR* DM_PPLIB is due to missing handling in powerplay

dpm for SI is available, while powerplay for SI is not, but display/amdgpu_dm uses some powerplay calls, where get_static_clock functions not available and the *ERROR* DM_PPLIB is due to missing handling in powerplay

Hi,

reporting about some progress made during the weekend, thanks to Sylvain feedback & suggestions.

I have rebased and updated the series on top of
https://cgit.freedesktop.org/~agd5f/linux/?h=amd-staging-drm-next

Here is the amd_dc_si branch:
https://github.com/maurossi/linux/tree/amd_dc_si (uploading)
NOTE: arch/x86/kernel/tsc.c changes for 4K display modes are not there, as they are not strictly needed for amd-gfx

Copying also Harry, Alex, Christian and Mike in order to get some objective and infallible
clues/feedbacks about blocking points and about "no care" items.

Please, also big things I may have missed.
M.

Quoted 5 lines · On Mon, Oct 8, 2018 at 11:23 PM <[email protected]> wrote:

Sylvain - I did hack a bit your patch set on amd-staging-drm-next to make it go through the asic init and I managed to get a x11 display with lines kind of garbled, but you can still understand easily what's on the screen.

I forgot to mention that since I'm gorgeously trying AMD DC also on Mullins I have reverted d9fda24 (""drm/amdgpu: Don't default to DC support for Kaveri and older")
because on Mullins I can boot with HDMI and HDMI-to-VGA converter

I was hoping for AMD DC being re-enabled for Kaveri and older, but I'm available to submit new version of specific patch if required.

Quoted 4 lines

Sylvain - ... The lines may be garbled in your driver code because, if I recall properly, "line buffer" programing in dce8 is not
the same than in dce6 (look for registers with the "LB" abbreviation). Or some slight differences in frame buffer tiling.

So the problem could be related to some kind of scan line or tiling buffer issue,
at the moment the dce_resouces model is grabbed "AS IS" from DCE8 registers/masks

Quoted 6 lines

Sylvain - I checked the kernel log, and like you said, I got errors in DM_PPLIB due to an invalid powerlevel and atombios/vbios table parsing regarding connectors. general dpm is in amdgpu(no DC) for SI, it means the DCE related dpm part in current SI amdgpu code path should be "copied" in DC. It is related very probably to the parsing of VBIOS/ATOMBIOS tables.

10-09 21:10:14.427 0 0 E :
[drm:dm_pp_get_static_clocks [amdgpu]] *ERROR* DM_PPLIB: invalid powerlevel state: 0!

NOTE: the error is the result of Powerplay dependency introduced by using AMD DC for SI
it's not fatal and it does not seem to affect performance in the Benchmarks

DOUBT: I think that it would make sense to set "power level 0" i.e. the "lower state" as safe default,
considering that powerplay smu6/hwmgr are not implemented for SI and smu7 CIK functions do not work,
the AS-IS dpm is the only available option. (and it seams to be working, looking at the framerates 250-280 in the V1 Vulkan benchmark)

Quoted 2 lines

10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!
10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!

NOTE: the warning also appears with Tonga and Vega, it is a Warning and does not seem to cause issues, so I would assume there is a default treatment in place,
is this related to missing encoder for drm crtc or to other kind of encoder?

Quoted 3 lines

Sylvain - Did add SI handling in some raven firmware loader function. In drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c, "load_dmcu_fw" function augmented with SI chip asic_type.

I've merged the change in the (v2) branch
https://github.com/maurossi/linux/tree/amd_dc_si

Quoted 6 lines

Sylvain - AFAIK, the real thing that you additionally get with DC is freesync. But freesync is actually going to be interesting only if displays are able to get their sync range lower bound to 0, and get significant power saving thanks to this. For the use case of very low display refresh rate I don't even think displayport or hdmi can do that, and be power friendly (you would have to retrain the link probably each time you send a framebuffer to the display).

If freesync is about reducing the framerate rate for power saving, provided that I've seen it be mentioned the first time for GCN 2nd generation, I'm not expecting freesync as a mandatory capability for the series.

Quoted 3 lines

Mauro -- dce60_resources was having too many building errors due to missing DCE6 macros in order to temporarily overcome the problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

Still to many building errors due to quite different registers naming, pointers to GPU register info (either in GPUopen or by means of listing the DCE6 vs DC8 differences),
or keeping the DCE6 is exactly like DCE8 as register changes are apparently not mission critical.

Quoted 2 lines

Mauro - dc/irq suffered the same problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

I could not update dc/irq VBI Vertical Blank Interrupt, because i cannot find the corresponding IRQ register in amdgpu/dce6 registers headers/mask

(quoted diff) +2 -2

-CRTC_VERTICAL_INTERRUPT0_CONTROL__CRTC_VERTICAL_INTERRUPT0_INT_ENABLE_MASK
+?seems trivial but who knows what is the corresponding in DCE6?
 
-CRTC_VERTICAL_INTERRUPT0_CONTROL__CRTC_VERTICAL_INTERRUPT0_CLEAR_MASK
+?seems trivial but who knows what is the corresponding in DCE6?

NOTE: If this is the very basic VBI Vertical Blank Interrupt signal handling, there should be dce6 registers/masks,
but some hint/documentation is necessary for me to find them.

Quoted 1 line

Mauro - gfx6 may require some ad hoc initialization, skipped for the moment

Are there ad hoc tiling settings which are necessary?

The OpenGLES and Vulkan radv apps I've tested:

Android CTS dEQP-VK only 85 tests failed over 220'000 Toy Zombies Lite
Sky Force Reloaded
V1 Benchmark Pro
GFXbench
Antutu 3D
Various OpenGLES demos

Here I'm planning to perform also dEQP-EGL, dEQP-GLES2, dEQP-GLES3 soon, but feedbacks from developers are very welcome and appreciated

Quoted 2 lines

Mauro - Hainan specific code requires review, as some documentation and code paths seem to point that famility may not have DCE6, please confirm

Hainan specifics were removed and are unsupported in the new serie as DCE6 physical module not available in Hainan parts. Unless the virtual_dce modules supports atomit, but I don't think so.

Quoted 1 line

Mauro - video decoding blocks code have not been touched

UVD and VCE firmwares and code changes for SI were necessary before the series and they are unrelated to AMD DC for SI patches.

Quoted 1 line

Mauro - dc/dce/dce_clock_source.{c,h} may be missing some SI/DCE6 specifics

In amd-staging-drm-next dce_clock_source is generic, SI specifics are not necessary anymore.

Quoted 3 lines

Sylvain - It _seems_ there is not that much additional work to do in order to make it properly work.

Ok, let's keep the momentum and continue tackle with x11 display problem and after that I'm runnign piglit no regression with x11 and with wayland too.

Quoted 1 line

Testing on x11,wayland or other ways

Any other testing tools worth a run?
In case there is some AMD/GPUopen testing tool with unit tests, please let me know
Kind regards

Mauro

Quoted 5 lines · Mauro Rossi

Hi,

reporting about some progress made during the weekend, thanks to Sylvain feedback & suggestions.

Quoted 7 lines

I have rebased and updated the series on top of
https://cgit.freedesktop.org/~agd5f/linux/?h=amd-staging-drm-next

Quoted 1 line

Quoted 1 line

Here is the amd_dc_si branch:

https://github.com/maurossi/linux/tree/amd_dc_si (uploading)
NOTE: arch/x86/kernel/tsc.c changes for 4K display modes are not there, as they are not strictly needed for amd-gfx

Quoted 13 lines

Copying also Harry, Alex, Christian and Mike in order to get some objective and infallible
clues/feedbacks about blocking points and about "no care" items.

Please, also big things I may have missed.
M.

Quoted 5 lines · On Mon, Oct 8, 2018 at 11:23 PM <[email protected]> wrote:

Sylvain - I did hack a bit your patch set on amd-staging-drm-next to make it go through the asic init and I managed to get a x11 display with lines kind of garbled, but you can still understand easily what's on the screen.

Quoted 8 lines

I forgot to mention that since I'm gorgeously trying AMD DC also on Mullins I have reverted d9fda24 (""drm/amdgpu: Don't default to DC support for Kaveri and older")
because on Mullins I can boot with HDMI and HDMI-to-VGA converter

I was hoping for AMD DC being re-enabled for Kaveri and older, but I'm available to submit new version of specific patch if required.

I still need to find time to get through your patchset properly. Just a quick note on this. There are Kabini/Kaveri ASICs with VGA connectors in the market, which the DC code doesn't support. If someone writes it we can re-enable it by default.

Either way, you can revert that patch for your tree or use amdgpu.dc=0 as long as you're aware that VGA won't work with amdgpu on such a kernel.

Harry

Quoted 10 lines

Quoted 4 lines

Sylvain - ... The lines may be garbled in your driver code because, if I recall properly, "line buffer" programing in dce8 is not
the same than in dce6 (look for registers with the "LB" abbreviation). Or some slight differences in frame buffer tiling.

So the problem could be related to some kind of scan line or tiling buffer issue,
at the moment the dce_resouces model is grabbed "AS IS" from DCE8 registers/masks

Quoted 15 lines

Quoted 6 lines

Sylvain - I checked the kernel log, and like you said, I got errors in DM_PPLIB due to an invalid powerlevel and atombios/vbios table parsing regarding connectors. general dpm is in amdgpu(no DC) for SI, it means the DCE related dpm part in current SI amdgpu code path should be "copied" in DC. It is related very probably to the parsing of VBIOS/ATOMBIOS tables.

10-09 21:10:14.427 0 0 E :
[drm:dm_pp_get_static_clocks [amdgpu]] *ERROR* DM_PPLIB: invalid powerlevel state: 0!

NOTE: the error is the result of Powerplay dependency introduced by using AMD DC for SI
it's not fatal and it does not seem to affect performance in the Benchmarks

Quoted 7 lines

DOUBT: I think that it would make sense to set "power level 0" i.e. the "lower state" as safe default,
considering that powerplay smu6/hwmgr are not implemented for SI and smu7 CIK functions do not work,
the AS-IS dpm is the only available option. (and it seams to be working, looking at the framerates 250-280 in the V1 Vulkan benchmark)

Quoted 105 lines

Quoted 4 lines

10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!
10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!

NOTE: the warning also appears with Tonga and Vega, it is a Warning and does not seem to cause issues, so I would assume there is a default treatment in place,
is this related to missing encoder for drm crtc or to other kind of encoder?

Quoted 3 lines

Sylvain - Did add SI handling in some raven firmware loader function. In drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c, "load_dmcu_fw" function augmented with SI chip asic_type.

I've merged the change in the (v2) branch
https://github.com/maurossi/linux/tree/amd_dc_si

Quoted 6 lines

Sylvain - AFAIK, the real thing that you additionally get with DC is freesync. But freesync is actually going to be interesting only if displays are able to get their sync range lower bound to 0, and get significant power saving thanks to this. For the use case of very low display refresh rate I don't even think displayport or hdmi can do that, and be power friendly (you would have to retrain the link probably each time you send a framebuffer to the display).

If freesync is about reducing the framerate rate for power saving, provided that I've seen it be mentioned the first time for GCN 2nd generation, I'm not expecting freesync as a mandatory capability for the series.

Quoted 3 lines

Mauro -- dce60_resources was having too many building errors due to missing DCE6 macros in order to temporarily overcome the problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

Still to many building errors due to quite different registers naming, pointers to GPU register info (either in GPUopen or by means of listing the DCE6 vs DC8 differences),
or keeping the DCE6 is exactly like DCE8 as register changes are apparently not mission critical.

Quoted 2 lines

Mauro - dc/irq suffered the same problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

I could not update dc/irq VBI Vertical Blank Interrupt, because i cannot find the corresponding IRQ register in amdgpu/dce6 registers headers/mask

(quoted diff) +2 -2

-CRTC_VERTICAL_INTERRUPT0_CONTROL__CRTC_VERTICAL_INTERRUPT0_INT_ENABLE_MASK
+?seems trivial but who knows what is the corresponding in DCE6?
 
-CRTC_VERTICAL_INTERRUPT0_CONTROL__CRTC_VERTICAL_INTERRUPT0_CLEAR_MASK
+?seems trivial but who knows what is the corresponding in DCE6?

NOTE: If this is the very basic VBI Vertical Blank Interrupt signal handling, there should be dce6 registers/masks,
but some hint/documentation is necessary for me to find them.

Quoted 1 line

Mauro - gfx6 may require some ad hoc initialization, skipped for the moment

Are there ad hoc tiling settings which are necessary?

The OpenGLES and Vulkan radv apps I've tested:

Android CTS dEQP-VK only 85 tests failed over 220'000 Toy Zombies Lite
Sky Force Reloaded
V1 Benchmark Pro
GFXbench
Antutu 3D
Various OpenGLES demos

Here I'm planning to perform also dEQP-EGL, dEQP-GLES2, dEQP-GLES3 soon, but feedbacks from developers are very welcome and appreciated

Quoted 2 lines

Mauro - Hainan specific code requires review, as some documentation and code paths seem to point that famility may not have DCE6, please confirm

Hainan specifics were removed and are unsupported in the new serie as DCE6 physical module not available in Hainan parts. Unless the virtual_dce modules supports atomit, but I don't think so.

Quoted 1 line

Mauro - video decoding blocks code have not been touched

UVD and VCE firmwares and code changes for SI were necessary before the series and they are unrelated to AMD DC for SI patches.

Quoted 1 line

Mauro - dc/dce/dce_clock_source.{c,h} may be missing some SI/DCE6 specifics

In amd-staging-drm-next dce_clock_source is generic, SI specifics are not necessary anymore.

Quoted 3 lines

Sylvain - It _seems_ there is not that much additional work to do in order to make it properly work.

Ok, let's keep the momentum and continue tackle with x11 display problem and after that I'm runnign piglit no regression with x11 and with wayland too.

Quoted 1 line

Testing on x11,wayland or other ways

Any other testing tools worth a run?
In case there is some AMD/GPUopen testing tool with unit tests, please let me know
Kind regards

Quoted 2 lines

Mauro

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Quoted 6 lines · Harry Wentland

Quoted 5 lines · Mauro Rossi

Hi,

reporting about some progress made during the weekend, thanks to Sylvain feedback & suggestions.

Quoted 7 lines

Quoted 7 lines

I have rebased and updated the series on top of
https://cgit.freedesktop.org/~agd5f/linux/?h=amd-staging-drm-next

Quoted 1 line

Quoted 1 line

Here is the amd_dc_si branch:

https://github.com/maurossi/linux/tree/amd_dc_si (uploading)
NOTE: arch/x86/kernel/tsc.c changes for 4K display modes are not there, as they are not strictly needed for amd-gfx

What updates do you have to your original series?

If there are substantial updates can you send a v2 of your series?

If not I'll go through the v1. The branch is great for review but email patches are easier for commenting.

Quoted 24 lines

Quoted 15 lines

Copying also Harry, Alex, Christian and Mike in order to get some objective and infallible
clues/feedbacks about blocking points and about "no care" items.

Please, also big things I may have missed.
M.

Quoted 5 lines · On Mon, Oct 8, 2018 at 11:23 PM <[email protected]> wrote:

Sylvain - I did hack a bit your patch set on amd-staging-drm-next to make it go through the asic init and I managed to get a x11 display with lines kind of garbled, but you can still understand easily what's on the screen.

Quoted 8 lines

I forgot to mention that since I'm gorgeously trying AMD DC also on Mullins I have reverted d9fda24 (""drm/amdgpu: Don't default to DC support for Kaveri and older")
because on Mullins I can boot with HDMI and HDMI-to-VGA converter

I was hoping for AMD DC being re-enabled for Kaveri and older, but I'm available to submit new version of specific patch if required.

Quoted 16 lines

I still need to find time to get through your patchset properly. Just a quick note on this. There are Kabini/Kaveri ASICs with VGA connectors in the market, which the DC code doesn't support. If someone writes it we can re-enable it by default.

Either way, you can revert that patch for your tree or use amdgpu.dc=0 as long as you're aware that VGA won't work with amdgpu on such a kernel.

Harry

Quoted 10 lines

Quoted 4 lines

Sylvain - ... The lines may be garbled in your driver code because, if I recall properly, "line buffer" programing in dce8 is not
the same than in dce6 (look for registers with the "LB" abbreviation). Or some slight differences in frame buffer tiling.

So the problem could be related to some kind of scan line or tiling buffer issue,
at the moment the dce_resouces model is grabbed "AS IS" from DCE8 registers/masks

We should probably update the DCE6 headers and use those. Sometimes register addresses change in subtle ways and cause problems later on that are hard to spot.

Quoted 15 lines

Quoted 15 lines

Quoted 6 lines

Sylvain - I checked the kernel log, and like you said, I got errors in DM_PPLIB due to an invalid powerlevel and atombios/vbios table parsing regarding connectors. general dpm is in amdgpu(no DC) for SI, it means the DCE related dpm part in current SI amdgpu code path should be "copied" in DC. It is related very probably to the parsing of VBIOS/ATOMBIOS tables.

10-09 21:10:14.427 0 0 E :
[drm:dm_pp_get_static_clocks [amdgpu]] *ERROR* DM_PPLIB: invalid powerlevel state: 0!

NOTE: the error is the result of Powerplay dependency introduced by using AMD DC for SI
it's not fatal and it does not seem to affect performance in the Benchmarks

Quoted 7 lines

Quoted 7 lines

DOUBT: I think that it would make sense to set "power level 0" i.e. the "lower state" as safe default,
considering that powerplay smu6/hwmgr are not implemented for SI and smu7 CIK functions do not work,
the AS-IS dpm is the only available option. (and it seams to be working, looking at the framerates 250-280 in the V1 Vulkan benchmark)

I wouldn't worry about this too much for now. DC really just wants powerplay. Might make sense to just silence the error print if things seem to work otherwise.

Great work in getting things up and running with DCE6 and thanks for sending patches to get this work upstreamed.

Harry

Quoted 112 lines

Quoted 98 lines

Quoted 4 lines

10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!
10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!

NOTE: the warning also appears with Tonga and Vega, it is a Warning and does not seem to cause issues, so I would assume there is a default treatment in place,
is this related to missing encoder for drm crtc or to other kind of encoder?

Quoted 3 lines

Sylvain - Did add SI handling in some raven firmware loader function. In drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c, "load_dmcu_fw" function augmented with SI chip asic_type.

I've merged the change in the (v2) branch
https://github.com/maurossi/linux/tree/amd_dc_si

Quoted 6 lines

Sylvain - AFAIK, the real thing that you additionally get with DC is freesync. But freesync is actually going to be interesting only if displays are able to get their sync range lower bound to 0, and get significant power saving thanks to this. For the use case of very low display refresh rate I don't even think displayport or hdmi can do that, and be power friendly (you would have to retrain the link probably each time you send a framebuffer to the display).

If freesync is about reducing the framerate rate for power saving, provided that I've seen it be mentioned the first time for GCN 2nd generation, I'm not expecting freesync as a mandatory capability for the series.

Quoted 3 lines

Mauro -- dce60_resources was having too many building errors due to missing DCE6 macros in order to temporarily overcome the problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

Still to many building errors due to quite different registers naming, pointers to GPU register info (either in GPUopen or by means of listing the DCE6 vs DC8 differences),
or keeping the DCE6 is exactly like DCE8 as register changes are apparently not mission critical.

Quoted 2 lines

Mauro - dc/irq suffered the same problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

I could not update dc/irq VBI Vertical Blank Interrupt, because i cannot find the corresponding IRQ register in amdgpu/dce6 registers headers/mask

(quoted diff) +2 -2

-CRTC_VERTICAL_INTERRUPT0_CONTROL__CRTC_VERTICAL_INTERRUPT0_INT_ENABLE_MASK
+?seems trivial but who knows what is the corresponding in DCE6?
 
-CRTC_VERTICAL_INTERRUPT0_CONTROL__CRTC_VERTICAL_INTERRUPT0_CLEAR_MASK
+?seems trivial but who knows what is the corresponding in DCE6?

NOTE: If this is the very basic VBI Vertical Blank Interrupt signal handling, there should be dce6 registers/masks,
but some hint/documentation is necessary for me to find them.

Quoted 1 line

Mauro - gfx6 may require some ad hoc initialization, skipped for the moment

Are there ad hoc tiling settings which are necessary?

The OpenGLES and Vulkan radv apps I've tested:

Android CTS dEQP-VK only 85 tests failed over 220'000 Toy Zombies Lite
Sky Force Reloaded
V1 Benchmark Pro
GFXbench
Antutu 3D
Various OpenGLES demos

Here I'm planning to perform also dEQP-EGL, dEQP-GLES2, dEQP-GLES3 soon, but feedbacks from developers are very welcome and appreciated

Quoted 2 lines

Mauro - Hainan specific code requires review, as some documentation and code paths seem to point that famility may not have DCE6, please confirm

Hainan specifics were removed and are unsupported in the new serie as DCE6 physical module not available in Hainan parts. Unless the virtual_dce modules supports atomit, but I don't think so.

Quoted 1 line

Mauro - video decoding blocks code have not been touched

UVD and VCE firmwares and code changes for SI were necessary before the series and they are unrelated to AMD DC for SI patches.

Quoted 1 line

Mauro - dc/dce/dce_clock_source.{c,h} may be missing some SI/DCE6 specifics

In amd-staging-drm-next dce_clock_source is generic, SI specifics are not necessary anymore.

Quoted 3 lines

Sylvain - It _seems_ there is not that much additional work to do in order to make it properly work.

Ok, let's keep the momentum and continue tackle with x11 display problem and after that I'm runnign piglit no regression with x11 and with wayland too.

Quoted 9 lines

Quoted 1 line

Testing on x11,wayland or other ways

Any other testing tools worth a run?
In case there is some AMD/GPUopen testing tool with unit tests, please let me know
Kind regards

Mauro

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Quoted 26 lines · Mauro Rossi

Hi,

reporting about some progress made during the weekend, thanks to Sylvain feedback & suggestions.

I have rebased and updated the series on top of
https://cgit.freedesktop.org/~agd5f/linux/?h=amd-staging-drm-next

Here is the amd_dc_si branch:
https://github.com/maurossi/linux/tree/amd_dc_si (uploading)
NOTE: arch/x86/kernel/tsc.c changes for 4K display modes are not there, as they are not strictly needed for amd-gfx

Copying also Harry, Alex, Christian and Mike in order to get some objective and infallible
clues/feedbacks about blocking points and about "no care" items.

Please, also big things I may have missed.
M.

Quoted 5 lines · On Mon, Oct 8, 2018 at 11:23 PM <[email protected]> wrote:

Sylvain - I did hack a bit your patch set on amd-staging-drm-next to make it go through the asic init and I managed to get a x11 display with lines kind of garbled, but you can still understand easily what's on the screen.

Quoted 8 lines

I forgot to mention that since I'm gorgeously trying AMD DC also on Mullins I have reverted d9fda24 (""drm/amdgpu: Don't default to DC support for Kaveri and older")
because on Mullins I can boot with HDMI and HDMI-to-VGA converter

I was hoping for AMD DC being re-enabled for Kaveri and older, but I'm available to submit new version of specific patch if required.

I still need to find time to get through your patchset properly. Just a quick note on this. There are Kabini/Kaveri ASICs with VGA connectors in the market, which the DC code doesn't support. If someone writes it we can re-enable it by default.

Either way, you can revert that patch for your tree or use amdgpu.dc=0 as long as you're aware that VGA won't work with amdgpu on such a kernel.

Harry

Quoted 11 lines

Quoted 5 lines

Sylvain - ... The lines may be garbled in your driver code because, if I recall properly, "line buffer" programing in dce8 is not
the same than in dce6 (look for registers with the "LB" abbreviation). Or some slight differences in frame buffer tiling.

So the problem could be related to some kind of scan line or tiling buffer issue,
at the moment the dce_resouces model is grabbed "AS IS" from DCE8 registers/masks

Quoted 24 lines

Quoted 6 lines

Sylvain - I checked the kernel log, and like you said, I got errors in DM_PPLIB due to an invalid powerlevel and atombios/vbios table parsing regarding connectors. general dpm is in amdgpu(no DC) for SI, it means the DCE related dpm part in current SI amdgpu code path should be "copied" in DC. It is related very probably to the parsing of VBIOS/ATOMBIOS tables.

10-09 21:10:14.427 0 0 E :
[drm:dm_pp_get_static_clocks [amdgpu]] *ERROR* DM_PPLIB: invalid powerlevel state: 0!

NOTE: the error is the result of Powerplay dependency introduced by using AMD DC for SI
it's not fatal and it does not seem to affect performance in the Benchmarks

DOUBT: I think that it would make sense to set "power level 0" i.e. the "lower state" as safe default,
considering that powerplay smu6/hwmgr are not implemented for SI and smu7 CIK functions do not work,
the AS-IS dpm is the only available option. (and it seams to be working, looking at the framerates 250-280 in the V1 Vulkan benchmark)

Quoted 8 lines

Quoted 2 lines

10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!
10-09 21:10:14.427     0     0 W [drm] dce110_link_encoder_construct: Failed to get encoder_cap_info from VBIOS with error code 4!

NOTE: the warning also appears with Tonga and Vega, it is a Warning and does not seem to cause issues, so I would assume there is a default treatment in place,
is this related to missing encoder for drm crtc or to other kind of encoder?

Quoted 15 lines

Quoted 3 lines

Sylvain - Did add SI handling in some raven firmware loader function. In drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c, "load_dmcu_fw" function augmented with SI chip asic_type.

I've merged the change in the (v2) branch
https://github.com/maurossi/linux/tree/amd_dc_si

Quoted 6 lines

Sylvain - AFAIK, the real thing that you additionally get with DC is freesync. But freesync is actually going to be interesting only if displays are able to get their sync range lower bound to 0, and get significant power saving thanks to this. For the use case of very low display refresh rate I don't even think displayport or hdmi can do that, and be power friendly (you would have to retrain the link probably each time you send a framebuffer to the display).

Quoted 8 lines

If freesync is about reducing the framerate rate for power saving, provided that I've seen it be mentioned the first time for GCN 2nd generation, I'm not expecting freesync as a mandatory capability for the series.

Quoted 3 lines

Mauro -- dce60_resources was having too many building errors due to missing DCE6 macros in order to temporarily overcome the problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

Quoted 16 lines

Still to many building errors due to quite different registers naming, pointers to GPU register info (either in GPUopen or by means of listing the DCE6 vs DC8 differences),
or keeping the DCE6 is exactly like DCE8 as register changes are apparently not mission critical.

Quoted 2 lines

Mauro - dc/irq suffered the same problem dce_8_0_{d,sh_mask}.h headers were used for the PoC

I could not update dc/irq VBI Vertical Blank Interrupt, because i cannot find the corresponding IRQ register in amdgpu/dce6 registers headers/mask

-CRTC_VERTICAL_INTERRUPT0_CONTROL__CRTC_VERTICAL_INTERRUPT0_INT_ENABLE_MASK +?seems trivial but who knows what is the corresponding in DCE6?

Quoted 7 lines

-CRTC_VERTICAL_INTERRUPT0_CONTROL__CRTC_VERTICAL_INTERRUPT0_CLEAR_MASK +?seems trivial but who knows what is the corresponding in DCE6?

NOTE: If this is the very basic VBI Vertical Blank Interrupt signal handling, there should be dce6 registers/masks,
but some hint/documentation is necessary for me to find them.

Quoted 4 lines

Quoted 2 lines

Mauro - gfx6 may require some ad hoc initialization, skipped for the moment

Are there ad hoc tiling settings which are necessary?

Quoted 14 lines

The OpenGLES and Vulkan radv apps I've tested:

Android CTS dEQP-VK only 85 tests failed over 220'000 Toy Zombies Lite
Sky Force Reloaded
V1 Benchmark Pro
GFXbench
Antutu 3D
Various OpenGLES demos

Here I'm planning to perform also dEQP-EGL, dEQP-GLES2, dEQP-GLES3 soon, but feedbacks from developers are very welcome and appreciated

Quoted 7 lines

Quoted 2 lines

Mauro - Hainan specific code requires review, as some documentation and code paths seem to point that famility may not have DCE6, please confirm

Hainan specifics were removed and are unsupported in the new serie as DCE6 physical module not available in Hainan parts. Unless the virtual_dce modules supports atomit, but I don't think so.

Quoted 4 lines

Quoted 1 line

Mauro - video decoding blocks code have not been touched

UVD and VCE firmwares and code changes for SI were necessary before the series and they are unrelated to AMD DC for SI patches.

Quoted 15 lines

Quoted 2 lines

Mauro - dc/dce/dce_clock_source.{c,h} may be missing some SI/DCE6 specifics

In amd-staging-drm-next dce_clock_source is generic, SI specifics are not necessary anymore.

Quoted 3 lines

Sylvain - It _seems_ there is not that much additional work to do in order to make it properly work.

Ok, let's keep the momentum and continue tackle with x11 display problem and after that I'm runnign piglit no regression with x11 and with wayland too.

Quoted 1 line

Testing on x11,wayland or other ways

Quoted 7 lines

Any other testing tools worth a run?
In case there is some AMD/GPUopen testing tool with unit tests, please let me know
Kind regards

Mauro