[PATCH 1/2] dt-bindings: soc: mediatek: add mdp3 mutex support for mt8186
allen-kh.cheng
allen-kh.cheng at mediatek.com
Mon Jul 11 04:28:33 PDT 2022
Hi Angelo,
On Fri, 2022-07-08 at 07:37 -0800, AngeloGioacchino Del Regno wrote:
> Il 08/07/22 13:58, allen-kh.cheng ha scritto:
> > Hi Angelo,
> >
> > On Fri, 2022-07-08 at 10:28 +0200, AngeloGioacchino Del Regno
> > wrote:
> > > Il 08/07/22 10:19, Matthias Brugger ha scritto:
> > > >
> > > >
> > > > On 08/07/2022 10:14, allen-kh.cheng wrote:
> > > > > Hi Angelo,
> > > > >
> > > > > On Thu, 2022-07-07 at 12:59 +0200, AngeloGioacchino Del Regno
> > > > > wrote:
> > > > > > Il 07/07/22 12:41, Matthias Brugger ha scritto:
> > > > > > >
> > > > > > >
> > > > > > > On 07/07/2022 10:52, AngeloGioacchino Del Regno wrote:
> > > > > > > > Il 05/07/22 14:26, Allen-KH Cheng ha scritto:
> > > > > > > > > Add mdp3 mutex compatible for mt8186 SoC.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Allen-KH Cheng <
> > > > > > > > > allen-kh.cheng at mediatek.com>
> > > > > > > > > Signed-off-by: Xiandong Wang <
> > > > > > > > > xiandong.wang at mediatek.com>
> > > > > > > >
> > > > > > > >
> > > > > > > > Please drop this commit. Adding a mdp3-mutex compatible
> > > > > > > > is
> > > > > > > > not
> > > > > > > > needed here.
> > > > > > > >
> > > > > > >
> > > > > > > Thanks for checking. We probably would need a fallback
> > > > > > > compatible.
> > > > > > > We can only know
> > > > > > > from the HW engineers that can confirm if the IP block is
> > > > > > > the
> > > > > > > same
> > > > > > > as the disp
> > > > > > > mutex or a different one.
> > > > > > >
> > > > > > > I'll drop both patches for now until things got clear.
> > > > > > >
> > > > > >
> > > > > > They're located in a different iospace from each other, but
> > > > > > either
> > > > > > the platform
> > > > > > data needs to *not be* joined together, or if they're
> > > > > > together,
> > > > > > I
> > > > > > would not like
> > > > > > having two different compatible strings for essentially the
> > > > > > same
> > > > > > thing.
> > > > > >
> > > > > > I would at this point prefer dropping '-disp' from
> > > > > > 'mediatek,mt8186-
> > > > > > disp-mutex'
> > > > > > so that we would be able to declare two 'mediatek,mt8186-
> > > > > > mutex'
> > > > > > in
> > > > > > devicetree...
> > > > > > ...or simply have two mediatek,mt8186-disp-mutex (although
> > > > > > logically
> > > > > > incorrect?).
> > > > > >
> > > > > > Cheers,
> > > > > > Angelo
> > > > > >
> > > > >
> > > > > Thanks for your opinion.
> > > > >
> > > > > They are two different hardwares for different address
> > > > > spaces.
> > > > >
> > > > > I think we drop '-disp' from 'mediatek,mt8186-disp-mutex'
> > > > > will be
> > > > > excessive because we also need to modify mutex node in all
> > > > > exited
> > > > > dts
> > > > > files.
> > > > >
> > > > > I prefer havingg two mediatek,mt8186-disp-mutex.
> > > > >
> > > > > ex:
> > > > > mutex: mutex at 14001000 {
> > > > > compatible = "mediatek,mt8186-disp-mutex";
> > > > > ..
> > > > > }
> > > > >
> > > > > mdp3_mutex0: mutex at 1b001000 {
> > > > > compatible = "mediatek,mt8186-disp-mutex";
> > > > > ...
> > > > > }
> > > > >
> > > > > What do you think?
> > > >
> > > > I think that's an acceptable solution.
> > > >
> > >
> > > I'm a bit undecided instead, now... because from what I
> > > understand
> > > now,
> > > the platform data fields
> > >
> > > .mutex_mod and .mutex_sof
> > >
> > > are *not valid* for mutex at 0x1b001000 but only for the instance
> > > at
> > > 0x14001000.
> > >
> > > If we go this way, at this point, we would be free (and allowed
> > > by
> > > the driver)
> > > to try to set these for 0x1b001000, and to try to set MDP3 table
> > > paths on
> > > 0x14001000, which is something that shouldn't be logically
> > > allowed,
> > > as the
> > > hardware does *not* support that.
> > >
> > > Unless I got that wrong, and these fields for MUTEX_MOD_DISP_xxxx
> > > do
> > > exist in
> > > the mutex instance at 0xb001000, in which case, I fully agree
> > > with
> > > Matthias.
> > >
> > > But otherwise, I have my doubts.
> > >
> > > Cheers,
> > > Angelo
> > >
> >
> > I got your point.
> >
> > The disp and mdp3 drivers work with the same data field beacase
> > 14001000 (disp mutex) would not use .mutex_table_mod and 1b001000
> > (mdp3
> > mutex) would not use .mutex_mod/.mutex_sof.
> >
> >
> > How about ...
> >
> > static const struct mtk_mutex_data mt8186_mutex_driver_data = {
> > .mutex_mod = mt8186_mutex_mod,
> > .mutex_sof = mt8186_mutex_sof,
> > .mutex_mod_reg = MT8183_MUTEX0_MOD0,
> > .mutex_sof_reg = MT8183_MUTEX0_SOF0,
> > };
> >
> > static const struct mtk_mutex_data mt8186_mutex_mdp_driver_data = {
> > .mutex_table_mod = mt8186_mutex_table_mod,
> > };
> >
> > { .compatible = "mediatek,mt8186-disp-mutex",
> > .data = &mt8186_mutex_driver_data},
> > { .compatible = "mediatek,mt8186-mdp3-mutex",
> > .data = &mt8186_mutex_mdp_driver_data},
> >
> >
> > mutex: mutex at 14001000 {
> > compatible = "mediatek,mt8186-disp-mutex";
> > ..
> > }
> > mdp3_mutex0: mutex at 1b001000 {
> > compatible = "mediatek,mt8186-mdp3-mutex";
> > ...
> > }
> >
> > Do you think that is feasible?
> >
>
> This makes a lot more sense to me.
>
> Though, you have to also add the mod and sof regs, because the mutex
> instance
> for MDP_MUTEX does have these registers, even though they are used
> for different
> mods/sofs.
>
> static const struct mtk_mutex_data mt8186_mutex_driver_data = {
> .mutex_mod = mt8186_mutex_mod,
> .mutex_sof = mt8186_mutex_sof,
> .mutex_mod_reg = MT8183_MUTEX0_MOD0,
> .mutex_sof_reg = MT8183_MUTEX0_SOF0,
> };
>
> static const struct mtk_mutex_data mt8186_mdp_mutex_driver_data = {
> .mutex_mod_reg = MT8183_MUTEX0_MOD0,
> .mutex_sof_reg = MT8183_MUTEX0_SOF0,
> .mutex_table_mod = mt8186_mdp_mutex_table_mod,
> };
>
> P.S.: Notice that mt8186_mdp_mutex_driver_data instead of
> mt8186_mutex_mdp_driver_data was chosen on purpose:
> like that, we're referencing to real block names.
>
> Regards,
> Angelo
>
Thanks for your suggestion.
According to our discussion, I will update in next version.
Best Regards,
Allen
> > Best Regards,
> > Allen
> >
> > > > Regards,
> > > > Matthias
> > > >
> > > > >
> > > > > Best regards,
> > > > > Allen
> > > > >
> > > > > > > Regards,
> > > > > > > Matthias
> > > > > > >
> > > > > > > > > ---
> > > > > > > > > .../devicetree/bindings/soc/mediatek/mediatek,mutex.y
> > > > > > > > > aml
> > > > > > > > > | 1 +
> > > > > > > > > 1 file changed, 1 insertion(+)
> > > > > > > > >
> > > > > > > > > diff --git
> > > > > > > > > a/Documentation/devicetree/bindings/soc/mediatek/medi
> > > > > > > > > atek
> > > > > > > > > ,mutex
> > > > > > > > > .yaml
> > > > > > > > > b/Documentation/devicetree/bindings/soc/mediatek/medi
> > > > > > > > > atek
> > > > > > > > > ,mutex
> > > > > > > > > .yaml
> > > > > > > > > index 627dcc3e8b32..234fa5dc07c2 100644
> > > > > > > > > ---
> > > > > > > > > a/Documentation/devicetree/bindings/soc/mediatek/medi
> > > > > > > > > atek
> > > > > > > > > ,mutex
> > > > > > > > > .yaml
> > > > > > > > > +++
> > > > > > > > > b/Documentation/devicetree/bindings/soc/mediatek/medi
> > > > > > > > > atek
> > > > > > > > > ,mutex
> > > > > > > > > .yaml
> > > > > > > > > @@ -30,6 +30,7 @@ properties:
> > > > > > > > > - mediatek,mt8173-disp-mutex
> > > > > > > > > - mediatek,mt8183-disp-mutex
> > > > > > > > > - mediatek,mt8186-disp-mutex
> > > > > > > > > + - mediatek,mt8186-mdp3-mutex
> > > > > > > > > - mediatek,mt8192-disp-mutex
> > > > > > > > > - mediatek,mt8195-disp-mutex
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > >
> > > > > >
>
>
>
More information about the linux-arm-kernel
mailing list