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. Regards