On 25.04.2024 1:25 PM, Marc Gonzalez wrote: > On 15/04/2024 21:56, Konrad Dybcio wrote: > >> On 4/10/24 13:13, Marc Gonzalez wrote: >> >>> Video decoder (venus) was broken on msm8998. >>> >>> PH found crude work-around: >>> Drop venus_sys_set_power_control() call. >>> >>> Bryan suggested proper fix: >>> Set required register offsets in venus GDSC structs. >>> Set HW_CTRL flag. >>> >>> GDSC = Globally Distributed Switch Controller >>> >>> Use same code as mmcc-msm8996 with: >>> s/venus_gdsc/video_top_gdsc/ >>> s/venus_core0_gdsc/video_subcore0_gdsc/ >>> s/venus_core1_gdsc/video_subcore1_gdsc/ >>> >>> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/include/dt-bindings/clock/msm-clocks-hwio-8996.h >>> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/include/dt-bindings/clock/msm-clocks-hwio-8998.h >>> >>> 0x1024 = MMSS_VIDEO GDSCR (undocumented) >>> 0x1028 = MMSS_VIDEO_CORE_CBCR >>> 0x1030 = MMSS_VIDEO_AHB_CBCR >>> 0x1034 = MMSS_VIDEO_AXI_CBCR >>> 0x1038 = MMSS_VIDEO_MAXI_CBCR >>> 0x1040 = MMSS_VIDEO_SUBCORE0 GDSCR (undocumented) >>> 0x1044 = MMSS_VIDEO_SUBCORE1 GDSCR (undocumented) >>> 0x1048 = MMSS_VIDEO_SUBCORE0_CBCR >>> 0x104c = MMSS_VIDEO_SUBCORE1_CBCR >>> >>> Suggested-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> >>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> >>> Signed-off-by: Marc Gonzalez <mgonzalez@xxxxxxxxxx> >>> --- >> >> [...] >> >> >>> static struct gdsc video_top_gdsc = { >>> .gdscr = 0x1024, >>> + .cxcs = (unsigned int []){ 0x1028, 0x1034, 0x1038 }, >>> + .cxc_count = 3, >> >> Marc, have you verified all three are necessary for stuff to work? >> >> I'd expect 0x1028/venus core to be absolutely necessary fwiw > > Considering the absence of public documentation, these register offsets > mostly boil down to cargo-cult programming. > > Thus, using different code on 8996 and 8998 risks provoking the wrath > of the embedded gods. Better, safer to cast the same incantations. I don't think many maintainers believe in such embedded gods and would rather see the thing tested and answered considering it's more or less deterministic.. Konrad
