<html><body><p>
<pre>
On Mon, 2025-03-24 at 18:12 +0100, 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/03/25 10:33, paul-pl.chen ha scritto:
> > From: Nancy Lin <nancy.lin@mediatek.com>
> >
> > Refactor SOF settings by adding mtk_mutex_get_output_comp_sof()
> > and extracting SOF logic from mtk_mutex_add_comp()
> > and mtk_mutex_remove_comp().
> >
> > - Added mtk_mutex_add_comp_sof() and mtk_mutex_remove_comp_sof()
> > for SOF settings.
> > - Reused the switch case for SOF IDs.
> > - Separated MOD and SOF logic.
> >
>
> This also needs more than one commit.
>
> The cleanups go in a commit doing only cleanups.
>
Hi AngeloGioacchino,
Based on your advice, I'm considering dividing the changes as follows:
Commit 1:refactor SOF settings for output components
- Added mtk_mutex_add_comp_sof() and mtk_mutex_remove_comp_sof()
for SOF settings.
- Separated MOD and SOF logic.
Commit 2:
- Reused the switch case for SOF IDs.
> > Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
> > Signed-off-by: Paul-pl Chen <paul-pl.chen@mediatek.com>
> > ---
> > drivers/soc/mediatek/mtk-mutex.c | 121 +++++++++++++++-----
> > -----
> > include/linux/soc/mediatek/mtk-mutex.h | 4 +
> > 2 files changed, 79 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-mutex.c
> > b/drivers/soc/mediatek/mtk-mutex.c
> > index aaa965d4b050..c026ac0e6969 100644
> > --- a/drivers/soc/mediatek/mtk-mutex.c
> > +++ b/drivers/soc/mediatek/mtk-mutex.c
> > @@ -853,43 +853,84 @@ void mtk_mutex_unprepare(struct mtk_mutex
> > *mutex)
> > }
> > EXPORT_SYMBOL_GPL(mtk_mutex_unprepare);
> >
> > -void mtk_mutex_add_comp(struct mtk_mutex *mutex,
> > - enum mtk_ddp_comp_id id)
> > +static int mtk_mutex_get_output_comp_sof(enum mtk_ddp_comp_id id)
> > {
> > - struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > - mutex[mutex->id]);
> > - unsigned int reg;
> > - unsigned int sof_id;
> > - unsigned int offset;
> > -
> > - WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > -
> > switch (id) {
> > case DDP_COMPONENT_DSI0:
> > - sof_id = MUTEX_SOF_DSI0;
> > - break;
> > + return MUTEX_SOF_DSI0;
> > case DDP_COMPONENT_DSI1:
> > - sof_id = MUTEX_SOF_DSI0;
> > - break;
> > + return MUTEX_SOF_DSI1;
> > case DDP_COMPONENT_DSI2:
> > - sof_id = MUTEX_SOF_DSI2;
> > - break;
> > + return MUTEX_SOF_DSI2;
> > case DDP_COMPONENT_DSI3:
> > - sof_id = MUTEX_SOF_DSI3;
> > - break;
> > + return MUTEX_SOF_DSI3;
> > case DDP_COMPONENT_DPI0:
> > - sof_id = MUTEX_SOF_DPI0;
> > - break;
> > + return MUTEX_SOF_DPI0;
> > case DDP_COMPONENT_DPI1:
> > - sof_id = MUTEX_SOF_DPI1;
> > - break;
> > + return MUTEX_SOF_DPI1;
> > case DDP_COMPONENT_DP_INTF0:
> > - sof_id = MUTEX_SOF_DP_INTF0;
> > - break;
> > + return MUTEX_SOF_DP_INTF0;
> > case DDP_COMPONENT_DP_INTF1:
> > - sof_id = MUTEX_SOF_DP_INTF1;
> > - break;
> > + return MUTEX_SOF_DP_INTF1;
> > default:
> > + break;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +void mtk_mutex_add_comp_sof(struct mtk_mutex *mutex, enum
> > mtk_ddp_comp_id id)
> > +{
> > + struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > + mutex[mutex->id]);
> > + int sof_id = mtk_mutex_get_output_comp_sof(id);
> > + unsigned int offset;
> > +
> > + if (sof_id < 0 || sof_id >= DDP_MUTEX_SOF_MAX)
> > + return;
> > +
> > + WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
> > + offset = DISP_REG_MUTEX_SOF(mtx->data->mutex_sof_reg, mutex-
> > >id);
> > +
> > + writel_relaxed(mtx->data->mutex_sof[sof_id],
> > + mtx->regs + offset);
>
> fits in one line
>
> OK, I will fix it in one line.
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mutex_add_comp_sof);
> > +
> > +void mtk_mutex_remove_comp_sof(struct mtk_mutex *mutex, enum
> > mtk_ddp_comp_id id)
> > +{
> > + struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > + mutex[mutex->id]);
> > + unsigned int reg;
> > + int sof_id = mtk_mutex_get_output_comp_sof(id);
> > + unsigned int offset;
> > +
> > + if (sof_id < 0 || sof_id >= DDP_MUTEX_SOF_MAX)
> > + return;
> > +
> > + WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
> > + offset = DISP_REG_MUTEX_SOF(mtx->data->mutex_sof_reg, mutex-
> > >id);
> > + reg = readl_relaxed(mtx->regs + offset);
> > + reg &= ~(1 << mtx->data->mutex_sof[id]);
> > +
> > + writel_relaxed(reg, mtx->regs + offset);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mutex_remove_comp_sof);
> > +
> > +void mtk_mutex_add_comp(struct mtk_mutex *mutex,
> > + enum mtk_ddp_comp_id id)
> > +{
> > + struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > + mutex[mutex->id]);
> > + unsigned int reg;
> > + unsigned int offset;
> > + bool is_output_comp = !!mtk_mutex_get_output_comp_sof(id);
> > +
> > + WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
>
> Looks like you can just do
>
> if (is_output_comp) {
> mtk_mutex_add_comp_sof(mutex, id);
> return;
> }
>
> if (mtx->data->mutex_mod[id] < 32) { .....etc
>
OK,
This is because the next patch [PATCH v2 08/15],
we have to consider using the condition of "mtx->data->need_sof_mod"
to set the register setting of DISP_REG_MUTEX_MOD.
> > + if (!is_output_comp) {
> > if (mtx->data->mutex_mod[id] < 32) {
> > offset = DISP_REG_MUTEX_MOD(mtx->data-
> > >mutex_mod_reg,
> > mutex->id);
> > @@ -902,12 +943,10 @@ void mtk_mutex_add_comp(struct mtk_mutex
> > *mutex,
> > reg |= 1 << (mtx->data->mutex_mod[id] - 32);
> > writel_relaxed(reg, mtx->regs + offset);
> > }
> > - return;
> > }
> >
> > - writel_relaxed(mtx->data->mutex_sof[sof_id],
> > - mtx->regs +
> > - DISP_REG_MUTEX_SOF(mtx->data->mutex_sof_reg,
> > mutex->id));
> > + if (is_output_comp)
> > + mtk_mutex_add_comp_sof(mutex, id);
> > }
> > EXPORT_SYMBOL_GPL(mtk_mutex_add_comp);
> >
> > @@ -918,24 +957,11 @@ void mtk_mutex_remove_comp(struct mtk_mutex
> > *mutex,
> > mutex[mutex->id]);
> > unsigned int reg;
> > unsigned int offset;
> > + bool is_output_comp = !!mtk_mutex_get_output_comp_sof(id);
> >
> > WARN_ON(&mtx->mutex[mutex->id] != mutex);
> >
> > - switch (id) {
> > - case DDP_COMPONENT_DSI0:
> > - case DDP_COMPONENT_DSI1:
> > - case DDP_COMPONENT_DSI2:
> > - case DDP_COMPONENT_DSI3:
> > - case DDP_COMPONENT_DPI0:
> > - case DDP_COMPONENT_DPI1:
> > - case DDP_COMPONENT_DP_INTF0:
> > - case DDP_COMPONENT_DP_INTF1:
> > - writel_relaxed(MUTEX_SOF_SINGLE_MODE,
> > - mtx->regs +
> > - DISP_REG_MUTEX_SOF(mtx->data-
> > >mutex_sof_reg,
> > - mutex->id));
> > - break;
> > - default:
> > + if (!is_output_comp) {
>
> same comment as before.
>
OK,
This is because the next patch [PATCH v2 08/15],
we have to consider using the condition of "mtx->data->need_sof_mod"
to set the register setting of DISP_REG_MUTEX_MOD.
Best regards,
Paul
> > if (mtx->data->mutex_mod[id] < 32) {
> > offset = DISP_REG_MUTEX_MOD(mtx->data-
> > >mutex_mod_reg,
> > mutex->id);
> > @@ -948,8 +974,11 @@ void mtk_mutex_remove_comp(struct mtk_mutex
> > *mutex,
> > reg &= ~(1 << (mtx->data->mutex_mod[id] -
> > 32));
> > writel_relaxed(reg, mtx->regs + offset);
> > }
> > - break;
> > }
> > +
> > + if (is_output_comp)
> > + mtk_mutex_remove_comp_sof(mutex, id);
> > +
>
> Regards,
> Angelo
>
>
</pre>
</p></body></html><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be
conveyed only to the designated recipient(s). Any use, dissemination,
distribution, printing, retaining or copying of this e-mail (including its
attachments) by unintended recipient(s) is strictly prohibited and may
be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender
immediately (by replying to this e-mail), delete any and all copies of
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->