Re: [PATCH v6 17/20] media: platform: mtk-mdp3: Use cmdq_pkt_jump_rel() without shift_pa

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

Hi Nicolas,

On Mon, 2025-06-02 at 11:37 -0400, Nicolas Dufresne wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Hi,
> 
> Le lundi 02 juin 2025 à 01:31 +0800, Jason-JH Lin a écrit :
> > With the removal of the shift_pa parameter,
> > cmdq_pkt_jump_rel_temp()
> > can be replaced by the new cmdq_pkt_jump_rel() without shift_pa.
> > 
> > Then, remove the cmdq_shift_pa variable in the mdp_dev structure
> > for
> > each mbox client.
> > 
> > Fixes: ade176534112 ("soc: mediatek: cmdq: Add parameter shift_pa
> > to cmdq_pkt_jump()")
> > Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> > ---
> >  drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c | 2 +-
> >  drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c | 2 --
> >  drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h | 1 -
> >  3 files changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c
> > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-
> > cmdq.c
> > index 7575ec376367..c35fe0e3a4d5 100644
> > --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c
> > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c
> > @@ -638,7 +638,7 @@ static struct mdp_cmdq_cmd
> > *mdp_cmdq_prepare(struct mdp_dev *mdp,
> >               goto err_free_path;
> >       }
> >       cmdq_pkt_eoc(&cmd->pkt);
> > -     cmdq_pkt_jump_rel_temp(&cmd->pkt, CMDQ_INST_SIZE, mdp-
> > >cmdq_shift_pa[pp_idx]);
> > +     cmdq_pkt_jump_rel(&cmd->pkt, CMDQ_INST_SIZE);
> 
> Did I miss something or this reverts the change made in 15/20 ? I'm
> also
> unsure if its correct to ask for backports of this with Fixes tag.
> Isn't this
> for MT8196, a new board ?

Oh, you didn't miss anything.
The reason for reverting `shift_pa` is that MT8196 has an extra
`mminfra_offset` (in PATCH 7/20) which is also used as driver data for
CMDQ driver. Considering that these HW settings are used to generate
GCE instructions, and there may be more and more such settings in the
future, I want to put these settings inside CMDQ driver as much as
possible to make CMDQ client drivers unaware of them.

Therefore, I proposed PATCH 3/20 and PATCH 4/20.
With these PATCHs, all CMDQ client drivers do not need to store and
bring any parameters like `shift_pa` or `mminfra_offset` to the
specified CMDQ APIs to generate instructions.

However, these patches have not been reviewed by other reviewers yet,
so we may have to wait a while to see if everyone agrees with this
approach. Then we can see if we need to revert these changes of MDP
driver.

By the way, what do you think of this approach? :-)

Regards,
Jason-JH Lin

> 
> Nicolas
> 
>