| Message ID | 20250409-topic-smem_dramc-v1-0-94d505cd5593@oss.qualcomm.com |
|---|---|
| Headers | show |
| Series | Retrieve information about DDR from SMEM | expand |
On 4/9/25 5:12 PM, Connor Abbott wrote: > On Wed, Apr 9, 2025 at 10:48 AM Konrad Dybcio <konradybcio@kernel.org> wrote: >> >> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >> >> The Highest Bank address Bit value can change based on memory type used. >> >> Attempt to retrieve it dynamically, and fall back to a reasonable >> default (the one used prior to this change) on error. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 22 ++++++++++++++++------ >> 1 file changed, 16 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> index 06465bc2d0b4b128cddfcfcaf1fe4252632b6777..0cc397378c99db35315209d0265ad9223e8b55c7 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> @@ -13,6 +13,7 @@ >> #include <linux/firmware/qcom/qcom_scm.h> >> #include <linux/pm_domain.h> >> #include <linux/soc/qcom/llcc-qcom.h> >> +#include <linux/soc/qcom/smem.h> >> >> #define GPU_PAS_ID 13 >> >> @@ -669,17 +670,22 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) >> static void a6xx_set_ubwc_config(struct msm_gpu *gpu) >> { >> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); >> + u32 hbb = qcom_smem_dram_get_hbb(); >> + u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1; >> + u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2); >> + u32 hbb_hi, hbb_lo; >> + >> /* >> * We subtract 13 from the highest bank bit (13 is the minimum value >> * allowed by hw) and write the lowest two bits of the remaining value >> * as hbb_lo and the one above it as hbb_hi to the hardware. >> */ >> - BUG_ON(adreno_gpu->ubwc_config.highest_bank_bit < 13); >> - u32 hbb = adreno_gpu->ubwc_config.highest_bank_bit - 13; >> - u32 hbb_hi = hbb >> 2; >> - u32 hbb_lo = hbb & 3; >> - u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1; >> - u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2); >> + if (hbb < 0) >> + hbb = adreno_gpu->ubwc_config.highest_bank_bit; > > No. The value we expose to userspace must match what we program. > You'll break VK_EXT_host_image_copy otherwise. I didn't know that was exposed to userspace. The value must be altered either way - ultimately, the hardware must receive the correct information. ubwc_config doesn't seem to be const, so I can edit it there if you like it better. Konrad
On Wed, Apr 9, 2025 at 11:22 AM Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote: > > On 4/9/25 5:12 PM, Connor Abbott wrote: > > On Wed, Apr 9, 2025 at 10:48 AM Konrad Dybcio <konradybcio@kernel.org> wrote: > >> > >> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > >> > >> The Highest Bank address Bit value can change based on memory type used. > >> > >> Attempt to retrieve it dynamically, and fall back to a reasonable > >> default (the one used prior to this change) on error. > >> > >> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > >> --- > >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 22 ++++++++++++++++------ > >> 1 file changed, 16 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >> index 06465bc2d0b4b128cddfcfcaf1fe4252632b6777..0cc397378c99db35315209d0265ad9223e8b55c7 100644 > >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >> @@ -13,6 +13,7 @@ > >> #include <linux/firmware/qcom/qcom_scm.h> > >> #include <linux/pm_domain.h> > >> #include <linux/soc/qcom/llcc-qcom.h> > >> +#include <linux/soc/qcom/smem.h> > >> > >> #define GPU_PAS_ID 13 > >> > >> @@ -669,17 +670,22 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) > >> static void a6xx_set_ubwc_config(struct msm_gpu *gpu) > >> { > >> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > >> + u32 hbb = qcom_smem_dram_get_hbb(); > >> + u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1; > >> + u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2); > >> + u32 hbb_hi, hbb_lo; > >> + > >> /* > >> * We subtract 13 from the highest bank bit (13 is the minimum value > >> * allowed by hw) and write the lowest two bits of the remaining value > >> * as hbb_lo and the one above it as hbb_hi to the hardware. > >> */ > >> - BUG_ON(adreno_gpu->ubwc_config.highest_bank_bit < 13); > >> - u32 hbb = adreno_gpu->ubwc_config.highest_bank_bit - 13; > >> - u32 hbb_hi = hbb >> 2; > >> - u32 hbb_lo = hbb & 3; > >> - u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1; > >> - u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2); > >> + if (hbb < 0) > >> + hbb = adreno_gpu->ubwc_config.highest_bank_bit; > > > > No. The value we expose to userspace must match what we program. > > You'll break VK_EXT_host_image_copy otherwise. > > I didn't know that was exposed to userspace. > > The value must be altered either way - ultimately, the hardware must > receive the correct information. ubwc_config doesn't seem to be const, > so I can edit it there if you like it better. > > Konrad Yes, you should be calling qcom_smem_dram_get_hbb() in a6xx_calc_ubwc_config(). You can already see there's a TODO there to plug it in. Connor
On Wed, Apr 9, 2025 at 11:40 AM Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote: > > On 4/9/25 5:30 PM, Connor Abbott wrote: > > On Wed, Apr 9, 2025 at 11:22 AM Konrad Dybcio > > <konrad.dybcio@oss.qualcomm.com> wrote: > >> > >> On 4/9/25 5:12 PM, Connor Abbott wrote: > >>> On Wed, Apr 9, 2025 at 10:48 AM Konrad Dybcio <konradybcio@kernel.org> wrote: > >>>> > >>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > >>>> > >>>> The Highest Bank address Bit value can change based on memory type used. > >>>> > >>>> Attempt to retrieve it dynamically, and fall back to a reasonable > >>>> default (the one used prior to this change) on error. > >>>> > >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > >>>> --- > >>>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 22 ++++++++++++++++------ > >>>> 1 file changed, 16 insertions(+), 6 deletions(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >>>> index 06465bc2d0b4b128cddfcfcaf1fe4252632b6777..0cc397378c99db35315209d0265ad9223e8b55c7 100644 > >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >>>> @@ -13,6 +13,7 @@ > >>>> #include <linux/firmware/qcom/qcom_scm.h> > >>>> #include <linux/pm_domain.h> > >>>> #include <linux/soc/qcom/llcc-qcom.h> > >>>> +#include <linux/soc/qcom/smem.h> > >>>> > >>>> #define GPU_PAS_ID 13 > >>>> > >>>> @@ -669,17 +670,22 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) > >>>> static void a6xx_set_ubwc_config(struct msm_gpu *gpu) > >>>> { > >>>> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > >>>> + u32 hbb = qcom_smem_dram_get_hbb(); > >>>> + u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1; > >>>> + u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2); > >>>> + u32 hbb_hi, hbb_lo; > >>>> + > >>>> /* > >>>> * We subtract 13 from the highest bank bit (13 is the minimum value > >>>> * allowed by hw) and write the lowest two bits of the remaining value > >>>> * as hbb_lo and the one above it as hbb_hi to the hardware. > >>>> */ > >>>> - BUG_ON(adreno_gpu->ubwc_config.highest_bank_bit < 13); > >>>> - u32 hbb = adreno_gpu->ubwc_config.highest_bank_bit - 13; > >>>> - u32 hbb_hi = hbb >> 2; > >>>> - u32 hbb_lo = hbb & 3; > >>>> - u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1; > >>>> - u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2); > >>>> + if (hbb < 0) > >>>> + hbb = adreno_gpu->ubwc_config.highest_bank_bit; > >>> > >>> No. The value we expose to userspace must match what we program. > >>> You'll break VK_EXT_host_image_copy otherwise. > >> > >> I didn't know that was exposed to userspace. > >> > >> The value must be altered either way - ultimately, the hardware must > >> receive the correct information. ubwc_config doesn't seem to be const, > >> so I can edit it there if you like it better. > >> > >> Konrad > > > > Yes, you should be calling qcom_smem_dram_get_hbb() in > > a6xx_calc_ubwc_config(). You can already see there's a TODO there to > > plug it in. > > Does this look good instead? > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index 0cc397378c99..ae8dbc250e6a 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -588,6 +588,8 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu) > > static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) > { > + u8 hbb; You can't make it u8 and then test for a negative value on error. Other than that, looks good. Connor > + > gpu->ubwc_config.rgb565_predicator = 0; > gpu->ubwc_config.uavflagprd_inv = 0; > gpu->ubwc_config.min_acc_len = 0; > @@ -636,7 +638,6 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) > adreno_is_a690(gpu) || > adreno_is_a730(gpu) || > adreno_is_a740_family(gpu)) { > - /* TODO: get ddr type from bootloader and use 2 for LPDDR4 */ > gpu->ubwc_config.highest_bank_bit = 16; > gpu->ubwc_config.amsbc = 1; > gpu->ubwc_config.rgb565_predicator = 1; > @@ -665,6 +666,13 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) > gpu->ubwc_config.highest_bank_bit = 14; > gpu->ubwc_config.min_acc_len = 1; > } > + > + /* Attempt to retrieve HBB data from SMEM, keep the above defaults in case of error */ > + hbb = qcom_smem_dram_get_hbb(); > + if (hbb < 0) > + return; > + > + gpu->ubwc_config.highest_bank_bit = hbb; > } > > static void a6xx_set_ubwc_config(struct msm_gpu *gpu) > > > Konrad
On 4/9/25 5:44 PM, Connor Abbott wrote: > On Wed, Apr 9, 2025 at 11:40 AM Konrad Dybcio > <konrad.dybcio@oss.qualcomm.com> wrote: >> >> On 4/9/25 5:30 PM, Connor Abbott wrote: >>> On Wed, Apr 9, 2025 at 11:22 AM Konrad Dybcio >>> <konrad.dybcio@oss.qualcomm.com> wrote: >>>> >>>> On 4/9/25 5:12 PM, Connor Abbott wrote: >>>>> On Wed, Apr 9, 2025 at 10:48 AM Konrad Dybcio <konradybcio@kernel.org> wrote: >>>>>> >>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>>> >>>>>> The Highest Bank address Bit value can change based on memory type used. >>>>>> >>>>>> Attempt to retrieve it dynamically, and fall back to a reasonable >>>>>> default (the one used prior to this change) on error. >>>>>> >>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>>> --- >>>>>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 22 ++++++++++++++++------ >>>>>> 1 file changed, 16 insertions(+), 6 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>>>>> index 06465bc2d0b4b128cddfcfcaf1fe4252632b6777..0cc397378c99db35315209d0265ad9223e8b55c7 100644 >>>>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>>>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>>>>> @@ -13,6 +13,7 @@ >>>>>> #include <linux/firmware/qcom/qcom_scm.h> >>>>>> #include <linux/pm_domain.h> >>>>>> #include <linux/soc/qcom/llcc-qcom.h> >>>>>> +#include <linux/soc/qcom/smem.h> >>>>>> >>>>>> #define GPU_PAS_ID 13 >>>>>> >>>>>> @@ -669,17 +670,22 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) >>>>>> static void a6xx_set_ubwc_config(struct msm_gpu *gpu) >>>>>> { >>>>>> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); >>>>>> + u32 hbb = qcom_smem_dram_get_hbb(); >>>>>> + u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1; >>>>>> + u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2); >>>>>> + u32 hbb_hi, hbb_lo; >>>>>> + >>>>>> /* >>>>>> * We subtract 13 from the highest bank bit (13 is the minimum value >>>>>> * allowed by hw) and write the lowest two bits of the remaining value >>>>>> * as hbb_lo and the one above it as hbb_hi to the hardware. >>>>>> */ >>>>>> - BUG_ON(adreno_gpu->ubwc_config.highest_bank_bit < 13); >>>>>> - u32 hbb = adreno_gpu->ubwc_config.highest_bank_bit - 13; >>>>>> - u32 hbb_hi = hbb >> 2; >>>>>> - u32 hbb_lo = hbb & 3; >>>>>> - u32 ubwc_mode = adreno_gpu->ubwc_config.ubwc_swizzle & 1; >>>>>> - u32 level2_swizzling_dis = !(adreno_gpu->ubwc_config.ubwc_swizzle & 2); >>>>>> + if (hbb < 0) >>>>>> + hbb = adreno_gpu->ubwc_config.highest_bank_bit; >>>>> >>>>> No. The value we expose to userspace must match what we program. >>>>> You'll break VK_EXT_host_image_copy otherwise. >>>> >>>> I didn't know that was exposed to userspace. >>>> >>>> The value must be altered either way - ultimately, the hardware must >>>> receive the correct information. ubwc_config doesn't seem to be const, >>>> so I can edit it there if you like it better. >>>> >>>> Konrad >>> >>> Yes, you should be calling qcom_smem_dram_get_hbb() in >>> a6xx_calc_ubwc_config(). You can already see there's a TODO there to >>> plug it in. >> >> Does this look good instead? >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> index 0cc397378c99..ae8dbc250e6a 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> @@ -588,6 +588,8 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu) >> >> static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu) >> { >> + u8 hbb; > > You can't make it u8 and then test for a negative value on error. Fair. I think it was u8 in a pre-release version of the patchset and it stuck in my mind.. though I'dve expected clang to warn me here.. > Other than that, looks good. Thanks, I'll change it in v2. Konrad
On 4/9/25 5:49 PM, Konrad Dybcio wrote: > On 4/9/25 5:44 PM, Dmitry Baryshkov wrote: >> On 09/04/2025 17:47, Konrad Dybcio wrote: >>> SMEM allows the OS to retrieve information about the DDR memory. >>> Among that information, is a semi-magic value called 'HBB', or Highest >>> Bank address Bit, which multimedia drivers (for hardware like Adreno >>> and MDSS) must retrieve in order to program the IP blocks correctly. >>> >>> This series introduces an API to retrieve that value, uses it in the >>> aforementioned programming sequences and exposes available DDR >>> frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More >>> information can be exposed in the future, as needed. >> >> I know that for some platforms HBB differs between GPU and DPU (as it's being programmed currently). Is there a way to check, which values are we going to program: >> >> - SM6115, SM6350, SM6375 (13 vs 14) SM6350 has INFO_V3 SM6375 has INFO_V3_WITH_14_FREQS >> - SC8180X (15 vs 16) So I overlooked the fact that DDR info v3 (e.g. on 8180) doesn't provide the HBB value.. Need to add some more sanity checks there. Maybe I can think up some fallback logic based on the DDR type reported. >> - QCM2290 (14 vs 15) I don't have one on hand, could you please give it a go on your RB1? I would assume both it and SM6115 also provide v3 though.. Konrad
On Fri, 11 Apr 2025 at 12:49, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote: > > On 4/9/25 5:49 PM, Konrad Dybcio wrote: > > On 4/9/25 5:44 PM, Dmitry Baryshkov wrote: > >> On 09/04/2025 17:47, Konrad Dybcio wrote: > >>> SMEM allows the OS to retrieve information about the DDR memory. > >>> Among that information, is a semi-magic value called 'HBB', or Highest > >>> Bank address Bit, which multimedia drivers (for hardware like Adreno > >>> and MDSS) must retrieve in order to program the IP blocks correctly. > >>> > >>> This series introduces an API to retrieve that value, uses it in the > >>> aforementioned programming sequences and exposes available DDR > >>> frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More > >>> information can be exposed in the future, as needed. > >> > >> I know that for some platforms HBB differs between GPU and DPU (as it's being programmed currently). Is there a way to check, which values are we going to program: > >> > >> - SM6115, SM6350, SM6375 (13 vs 14) > > SM6350 has INFO_V3 > SM6375 has INFO_V3_WITH_14_FREQS I'm not completely sure what you mean here. I pointed out that these platforms disagreed upon the HBB value between the DPU/msm_mdss.c and a6xx_gpu.c. In some cases (a610/SM6115 and a619/SM6350) that was intentional to fix screen corruption issues. I don't remember if it was the case for QCM2290 or not. > > >> - SC8180X (15 vs 16) > > So I overlooked the fact that DDR info v3 (e.g. on 8180) doesn't provide > the HBB value.. Need to add some more sanity checks there. > > Maybe I can think up some fallback logic based on the DDR type reported. > > >> - QCM2290 (14 vs 15) > > I don't have one on hand, could you please give it a go on your RB1? > I would assume both it and SM6115 also provide v3 though.. > > Konrad
On 4/11/25 11:57 AM, Dmitry Baryshkov wrote: > On Fri, 11 Apr 2025 at 12:49, Konrad Dybcio > <konrad.dybcio@oss.qualcomm.com> wrote: >> >> On 4/9/25 5:49 PM, Konrad Dybcio wrote: >>> On 4/9/25 5:44 PM, Dmitry Baryshkov wrote: >>>> On 09/04/2025 17:47, Konrad Dybcio wrote: >>>>> SMEM allows the OS to retrieve information about the DDR memory. >>>>> Among that information, is a semi-magic value called 'HBB', or Highest >>>>> Bank address Bit, which multimedia drivers (for hardware like Adreno >>>>> and MDSS) must retrieve in order to program the IP blocks correctly. >>>>> >>>>> This series introduces an API to retrieve that value, uses it in the >>>>> aforementioned programming sequences and exposes available DDR >>>>> frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More >>>>> information can be exposed in the future, as needed. >>>> >>>> I know that for some platforms HBB differs between GPU and DPU (as it's being programmed currently). Is there a way to check, which values are we going to program: >>>> >>>> - SM6115, SM6350, SM6375 (13 vs 14) >> >> SM6350 has INFO_V3 >> SM6375 has INFO_V3_WITH_14_FREQS > > I'm not completely sure what you mean here. I pointed out that these > platforms disagreed upon the HBB value between the DPU/msm_mdss.c and > a6xx_gpu.c. > In some cases (a610/SM6115 and a619/SM6350) that was intentional to > fix screen corruption issues. I don't remember if it was the case for > QCM2290 or not. As I said below, I couldn't get a good answer yet, as the magic value is not provided explicitly and I'll hopefully be able to derive it from the available data Konrad > >> >>>> - SC8180X (15 vs 16) >> >> So I overlooked the fact that DDR info v3 (e.g. on 8180) doesn't provide >> the HBB value.. Need to add some more sanity checks there. >> >> Maybe I can think up some fallback logic based on the DDR type reported. >> >>>> - QCM2290 (14 vs 15) >> >> I don't have one on hand, could you please give it a go on your RB1? >> I would assume both it and SM6115 also provide v3 though.. >> >> Konrad > > >
On Fri, Apr 11, 2025 at 12:03:03PM +0200, Konrad Dybcio wrote: > On 4/11/25 11:57 AM, Dmitry Baryshkov wrote: > > On Fri, 11 Apr 2025 at 12:49, Konrad Dybcio > > <konrad.dybcio@oss.qualcomm.com> wrote: > >> > >> On 4/9/25 5:49 PM, Konrad Dybcio wrote: > >>> On 4/9/25 5:44 PM, Dmitry Baryshkov wrote: > >>>> On 09/04/2025 17:47, Konrad Dybcio wrote: > >>>>> SMEM allows the OS to retrieve information about the DDR memory. > >>>>> Among that information, is a semi-magic value called 'HBB', or Highest > >>>>> Bank address Bit, which multimedia drivers (for hardware like Adreno > >>>>> and MDSS) must retrieve in order to program the IP blocks correctly. > >>>>> > >>>>> This series introduces an API to retrieve that value, uses it in the > >>>>> aforementioned programming sequences and exposes available DDR > >>>>> frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More > >>>>> information can be exposed in the future, as needed. > >>>> > >>>> I know that for some platforms HBB differs between GPU and DPU (as it's being programmed currently). Is there a way to check, which values are we going to program: > >>>> > >>>> - SM6115, SM6350, SM6375 (13 vs 14) > >> > >> SM6350 has INFO_V3 > >> SM6375 has INFO_V3_WITH_14_FREQS > > > > I'm not completely sure what you mean here. I pointed out that these > > platforms disagreed upon the HBB value between the DPU/msm_mdss.c and > > a6xx_gpu.c. > > In some cases (a610/SM6115 and a619/SM6350) that was intentional to > > fix screen corruption issues. I don't remember if it was the case for > > QCM2290 or not. > > As I said below, I couldn't get a good answer yet, as the magic value > is not provided explicitly and I'll hopefully be able to derive it from > the available data I see... Is this data even supposed to be poked into? The foo_WITH_bar types doesn't sound like a very stable API. > > Konrad > > > > >> > >>>> - SC8180X (15 vs 16) > >> > >> So I overlooked the fact that DDR info v3 (e.g. on 8180) doesn't provide > >> the HBB value.. Need to add some more sanity checks there. > >> > >> Maybe I can think up some fallback logic based on the DDR type reported. > >> > >>>> - QCM2290 (14 vs 15) > >> > >> I don't have one on hand, could you please give it a go on your RB1? > >> I would assume both it and SM6115 also provide v3 though.. > >> > >> Konrad > > > > > >
On 4/11/25 12:50 PM, Dmitry Baryshkov wrote: > On Fri, Apr 11, 2025 at 12:03:03PM +0200, Konrad Dybcio wrote: >> On 4/11/25 11:57 AM, Dmitry Baryshkov wrote: >>> On Fri, 11 Apr 2025 at 12:49, Konrad Dybcio >>> <konrad.dybcio@oss.qualcomm.com> wrote: >>>> >>>> On 4/9/25 5:49 PM, Konrad Dybcio wrote: >>>>> On 4/9/25 5:44 PM, Dmitry Baryshkov wrote: >>>>>> On 09/04/2025 17:47, Konrad Dybcio wrote: >>>>>>> SMEM allows the OS to retrieve information about the DDR memory. >>>>>>> Among that information, is a semi-magic value called 'HBB', or Highest >>>>>>> Bank address Bit, which multimedia drivers (for hardware like Adreno >>>>>>> and MDSS) must retrieve in order to program the IP blocks correctly. >>>>>>> >>>>>>> This series introduces an API to retrieve that value, uses it in the >>>>>>> aforementioned programming sequences and exposes available DDR >>>>>>> frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More >>>>>>> information can be exposed in the future, as needed. >>>>>> >>>>>> I know that for some platforms HBB differs between GPU and DPU (as it's being programmed currently). Is there a way to check, which values are we going to program: >>>>>> >>>>>> - SM6115, SM6350, SM6375 (13 vs 14) >>>> >>>> SM6350 has INFO_V3 >>>> SM6375 has INFO_V3_WITH_14_FREQS >>> >>> I'm not completely sure what you mean here. I pointed out that these >>> platforms disagreed upon the HBB value between the DPU/msm_mdss.c and >>> a6xx_gpu.c. >>> In some cases (a610/SM6115 and a619/SM6350) that was intentional to >>> fix screen corruption issues. I don't remember if it was the case for >>> QCM2290 or not. >> >> As I said below, I couldn't get a good answer yet, as the magic value >> is not provided explicitly and I'll hopefully be able to derive it from >> the available data > > I see... > Is this data even supposed to be poked into? The foo_WITH_bar types > doesn't sound like a very stable API. Yeah, it was designed with both the producer and consumer being part of a single codebase, always having the data structures in sync.. Konrad
On Fri, Apr 11, 2025 at 12:52:32PM +0200, Konrad Dybcio wrote: > On 4/11/25 12:50 PM, Dmitry Baryshkov wrote: > > On Fri, Apr 11, 2025 at 12:03:03PM +0200, Konrad Dybcio wrote: > >> On 4/11/25 11:57 AM, Dmitry Baryshkov wrote: > >>> On Fri, 11 Apr 2025 at 12:49, Konrad Dybcio > >>> <konrad.dybcio@oss.qualcomm.com> wrote: > >>>> > >>>> On 4/9/25 5:49 PM, Konrad Dybcio wrote: > >>>>> On 4/9/25 5:44 PM, Dmitry Baryshkov wrote: > >>>>>> On 09/04/2025 17:47, Konrad Dybcio wrote: > >>>>>>> SMEM allows the OS to retrieve information about the DDR memory. > >>>>>>> Among that information, is a semi-magic value called 'HBB', or Highest > >>>>>>> Bank address Bit, which multimedia drivers (for hardware like Adreno > >>>>>>> and MDSS) must retrieve in order to program the IP blocks correctly. > >>>>>>> > >>>>>>> This series introduces an API to retrieve that value, uses it in the > >>>>>>> aforementioned programming sequences and exposes available DDR > >>>>>>> frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More > >>>>>>> information can be exposed in the future, as needed. > >>>>>> > >>>>>> I know that for some platforms HBB differs between GPU and DPU (as it's being programmed currently). Is there a way to check, which values are we going to program: > >>>>>> > >>>>>> - SM6115, SM6350, SM6375 (13 vs 14) > >>>> > >>>> SM6350 has INFO_V3 > >>>> SM6375 has INFO_V3_WITH_14_FREQS > >>> > >>> I'm not completely sure what you mean here. I pointed out that these > >>> platforms disagreed upon the HBB value between the DPU/msm_mdss.c and > >>> a6xx_gpu.c. > >>> In some cases (a610/SM6115 and a619/SM6350) that was intentional to > >>> fix screen corruption issues. I don't remember if it was the case for > >>> QCM2290 or not. > >> > >> As I said below, I couldn't get a good answer yet, as the magic value > >> is not provided explicitly and I'll hopefully be able to derive it from > >> the available data > > > > I see... > > Is this data even supposed to be poked into? The foo_WITH_bar types > > doesn't sound like a very stable API. > > Yeah, it was designed with both the producer and consumer being part > of a single codebase, always having the data structures in sync.. I feel somewhat worried about parsing those structures then. But... the only viable alternative is to have an in-kernel list of possible platform configurations and parse the /memory@foo/ddr_device_type property.
On 4/14/25 2:28 PM, Dmitry Baryshkov wrote: > On Fri, Apr 11, 2025 at 12:52:32PM +0200, Konrad Dybcio wrote: >> On 4/11/25 12:50 PM, Dmitry Baryshkov wrote: >>> On Fri, Apr 11, 2025 at 12:03:03PM +0200, Konrad Dybcio wrote: >>>> On 4/11/25 11:57 AM, Dmitry Baryshkov wrote: >>>>> On Fri, 11 Apr 2025 at 12:49, Konrad Dybcio >>>>> <konrad.dybcio@oss.qualcomm.com> wrote: >>>>>> >>>>>> On 4/9/25 5:49 PM, Konrad Dybcio wrote: >>>>>>> On 4/9/25 5:44 PM, Dmitry Baryshkov wrote: >>>>>>>> On 09/04/2025 17:47, Konrad Dybcio wrote: >>>>>>>>> SMEM allows the OS to retrieve information about the DDR memory. >>>>>>>>> Among that information, is a semi-magic value called 'HBB', or Highest >>>>>>>>> Bank address Bit, which multimedia drivers (for hardware like Adreno >>>>>>>>> and MDSS) must retrieve in order to program the IP blocks correctly. >>>>>>>>> >>>>>>>>> This series introduces an API to retrieve that value, uses it in the >>>>>>>>> aforementioned programming sequences and exposes available DDR >>>>>>>>> frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More >>>>>>>>> information can be exposed in the future, as needed. >>>>>>>> >>>>>>>> I know that for some platforms HBB differs between GPU and DPU (as it's being programmed currently). Is there a way to check, which values are we going to program: >>>>>>>> >>>>>>>> - SM6115, SM6350, SM6375 (13 vs 14) >>>>>> >>>>>> SM6350 has INFO_V3 >>>>>> SM6375 has INFO_V3_WITH_14_FREQS >>>>> >>>>> I'm not completely sure what you mean here. I pointed out that these >>>>> platforms disagreed upon the HBB value between the DPU/msm_mdss.c and >>>>> a6xx_gpu.c. >>>>> In some cases (a610/SM6115 and a619/SM6350) that was intentional to >>>>> fix screen corruption issues. I don't remember if it was the case for >>>>> QCM2290 or not. >>>> >>>> As I said below, I couldn't get a good answer yet, as the magic value >>>> is not provided explicitly and I'll hopefully be able to derive it from >>>> the available data >>> >>> I see... >>> Is this data even supposed to be poked into? The foo_WITH_bar types >>> doesn't sound like a very stable API. >> >> Yeah, it was designed with both the producer and consumer being part >> of a single codebase, always having the data structures in sync.. > > I feel somewhat worried about parsing those structures then. But... the > only viable alternative is to have an in-kernel list of possible > platform configurations and parse the /memory@foo/ddr_device_type > property. Well, this is where that property's value comes from.. Konrad
SMEM allows the OS to retrieve information about the DDR memory. Among that information, is a semi-magic value called 'HBB', or Highest Bank address Bit, which multimedia drivers (for hardware like Adreno and MDSS) must retrieve in order to program the IP blocks correctly. This series introduces an API to retrieve that value, uses it in the aforementioned programming sequences and exposes available DDR frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More information can be exposed in the future, as needed. Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> --- Konrad Dybcio (4): soc: qcom: Expose DDR data from SMEM drm/msm/a5xx: Get HBB dynamically, if available drm/msm/a6xx: Get HBB dynamically, if available drm/msm/mdss: Get HBB dynamically, if available drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 13 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 22 ++- drivers/gpu/drm/msm/msm_mdss.c | 35 ++++- drivers/soc/qcom/Makefile | 3 +- drivers/soc/qcom/smem.c | 14 +- drivers/soc/qcom/smem.h | 9 ++ drivers/soc/qcom/smem_dramc.c | 287 ++++++++++++++++++++++++++++++++++ include/linux/soc/qcom/smem.h | 4 + 8 files changed, 371 insertions(+), 16 deletions(-) --- base-commit: 46086739de22d72319e37c37a134d32db52e1c5c change-id: 20250409-topic-smem_dramc-6467187ac865 Best regards,