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. >On Mon, Oct 8, 2018 at 11:23 PM <sylvain.bertrand@xxxxxxxxx> 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. > 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 > > 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) >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? > 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 > 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. > 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. > 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? -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. >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 > 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. > 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. >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. > 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. > 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 amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx