Hi Harry, thanks a lot for response, On Mon, Oct 15, 2018 at 11:19 PM Harry Wentland <harry.wentland@xxxxxxx> wrote: > > On 2018-10-15 5:06 p.m., Harry Wentland wrote: > > On 2018-10-14 5:47 p.m., Mauro Rossi wrote: > >> 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 > >> > > > What updates do you have to your original series? > > If there are substantial updates can you send a v2 of your series? The new v2 series is rebased on amd-staging-drm-next with following benefits: - dce_clocks chipset families cases are not required anymore - dcmu fw load for DCN1 has some SI cases added - it is ready to be applied in amd-staging-drm-next Not much changes compared to v1, but I'm about to submit v2 very soon. > > If not I'll go through the v1. The branch is great for review but email patches are easier for commenting. This morning I've launched a build on v2 series, on amd-staging-drm-next branch rebased on top of 4.19rc8 just to test it builds fine. I'm sending the v2 series this evening and review may proceed on v2 amd-staging-drm-next context. Mauro > > > >> 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. > >> > > > > 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 > > > >>> 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. > > >>> > >>> 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) > >> > > 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 > > >> > >> > >>> 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 > > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
