[PATCH v1] memory: mtk-smi: Add ostd setting for mt8186

[Friday Yang (杨阳)]

On Wed, 2025-05-21 at 11:26 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 21/05/25 11:16, Friday Yang ha scritto:
> > Add initial ostd setting for mt8186. All the settings come from DE.
> > These settings help adjust Multimedia HW's bandwidth limits to
> > achieve
> > a balanced bandwidth requirement. Without this, the VENC HW works
> > abnormal while stress testing.
> > 
> > Fixes: 86a010bfc739 ("memory: mtk-smi: mt8186: Add smi support")
> > Signed-off-by: Friday Yang <friday.yang at mediatek.com>
> 
> I agree about this commit and you can get my
> 
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno at collabora.com>
> 
> ...but I still have a question.
> 
> This driver is getting lots of those big OSTD arrays, and I can
> foresee this
> getting bigger and bigger with every new SoC getting supported in
> there.
> 
> I'd like to understand how we can improve that, hence, can you please
> describe
> how the OSTD values are calculated and how are they limiting the
> bandwidth?
> 
> I'm thinking that we can do something such that we get this runtime
> calculated
> instead of just holding fixed values, so that we may eventually
> replace all those
> big arrays with just a few values (foreseeing 3-4 values) and
> performing a big
> cleanup (which may bring further improvements in the future).
> 
> Cheers,
> Angelo
> 

Thank you for your comments.
OSTD indicates the number of commands that the SMI LARB has received
from masters but has not yet responded to from DRAM. The SMI LARB has a
set of registers for each port to set the OSTD value. For example, if
OSTD is set to 3, then the hardware allows 3 commands in the FIFO for
that LARB port.

For each SoC, the ESL DE simulates application scenarios and provides
an initial OSTD setting to the software. This is just the initial
configuration. We have MMQoS software that dynamically adjusts the
bandwidth and sets the OSTD-related registers of SMI. In the MT8196,
MMQoS will send upstream. However, in some projects, the MMQoS module
is not enabled, so we need to provide this initial OSTD setting to
achieve a balanced bandwidth requirement. Since we do not have a
formula to calculate OSTD, it is difficult for SMI to calculate OSTD at
runtime.

You are right, the SMI driver could become larger if we have to add
this OSTD array. How about adding an OSTD setting in the device tree?
For example, we could add a mediatek,smi-ostd array property for each
SMI LARB and SMI common. This would be more feasible for us. Is this
acceptable to you?

Best regard
Friday Yang

> > ---
> >   drivers/memory/mtk-smi.c | 33 +++++++++++++++++++++++++++++++++
> >   1 file changed, 33 insertions(+)
> > 
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index c086c22511f7..733e22f695ab 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -320,6 +320,38 @@ static const u8
> > mtk_smi_larb_mt6893_ostd[][SMI_LARB_PORT_NR_MAX] = {
> >       [20] = {0x9, 0x9, 0x5, 0x5, 0x1, 0x1},
> >   };
> > 
> > +static const u8 mtk_smi_larb_mt8186_ostd[][SMI_LARB_PORT_NR_MAX] =
> > {
> > +     [0] = {0x2, 0x1, 0x8, 0x1,},
> > +     [1] = {0x1, 0x3, 0x1, 0x1,},
> > +     [2] = {0x6, 0x1, 0x4, 0x1,},
> > +     [3] = {},
> > +     [4] = {0xf, 0x1, 0x5, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1,
> > +            0x1, 0x1, 0x1,},
> > +     [5] = {},
> > +     [6] = {},
> > +     [7] = {0x1, 0x3, 0x1, 0x1, 0x1, 0x3, 0x2, 0xd, 0x7, 0x5, 0x3,
> > +            0x1, 0x5,},
> > +     [8] = {0x1, 0x2, 0x2,},
> > +     [9] = {0x9, 0x7, 0xf, 0x8, 0x1, 0x8, 0x9, 0x3, 0x3, 0xb, 0x7,
> > 0x4,
> > +            0x9, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1,
> > 0x1,
> > +            0x1, 0x1, 0x1, 0x1, 0x1,},
> > +     [10] = {},
> > +     [11] = {0x9, 0x7, 0xf, 0x8, 0x1, 0x8, 0x9, 0x3, 0x3, 0xb,
> > 0x7, 0x4,
> > +             0x9, 0x1, 0x1, 0x1, 0x1, 0x1, 0x8, 0x7, 0x7, 0x1,
> > 0x6, 0x2,
> > +             0xf, 0x8, 0x1, 0x1, 0x1,},
> > +     [12] = {},
> > +     [13] = {0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x6, 0x6, 0x6, 0x1,
> > 0x1, 0x1,},
> > +     [14] = {0x1, 0x1, 0x1, 0x1, 0x1, 0x1,},
> > +     [15] = {},
> > +     [16] = {0x28, 0x14, 0x2, 0xc, 0x18, 0x1, 0x14, 0x1, 0x4, 0x4,
> > 0x4,
> > +             0x2, 0x4, 0x2, 0x8, 0x4, 0x4,},
> > +     [17] = {0x28, 0x14, 0x2, 0xc, 0x18, 0x1, 0x14, 0x1, 0x4, 0x4,
> > 0x4,
> > +             0x2, 0x4, 0x2, 0x8, 0x4, 0x4,},
> > +     [18] = {},
> > +     [19] = {0x1, 0x1, 0x1, 0x1,},
> > +     [20] = {0x2, 0x2, 0x2, 0x2, 0x1, 0x1,},
> > +};
> > +
> >   static const u8 mtk_smi_larb_mt8188_ostd[][SMI_LARB_PORT_NR_MAX]
> > = {
> >       [0] = {0x02, 0x18, 0x22, 0x22, 0x01, 0x02, 0x0a,},
> >       [1] = {0x12, 0x02, 0x14, 0x14, 0x01, 0x18, 0x0a,},
> > @@ -491,6 +523,7 @@ static const struct mtk_smi_larb_gen
> > mtk_smi_larb_mt8183 = {
> >   static const struct mtk_smi_larb_gen mtk_smi_larb_mt8186 = {
> >       .config_port                =
> > mtk_smi_larb_config_port_gen2_general,
> >       .flags_general              = MTK_SMI_FLAG_SLEEP_CTL,
> > +     .ostd                       = mtk_smi_larb_mt8186_ostd,
> >   };
> > 
> >   static const struct mtk_smi_larb_gen mtk_smi_larb_mt8188 = {
> > --
> > 2.46.0
> > 
> 
>