On Mon, 2023-11-06 at 01:33 +0000, CK Hu (胡俊光) wrote:
> On Sun, 2023-11-05 at 13:18 +0000, Jason-JH Lin (林睿祥) wrote:
> > Hi CK,
> >
> > On Thu, 2023-10-26 at 10:07 +0000, CK Hu (胡俊光) wrote:
> > > Hi, Jason:
> > >
> > > On Mon, 2023-10-23 at 12:45 +0800, Jason-JH.Lin wrote:
> > > > Add secure layer config support for ovl.
> > > >
> > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@xxxxxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 3 ++
> > > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 31
> > > > +++++++++++++++++--
> > > > .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 12 +++++++
> > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 ++
> > > > 4 files changed, 46 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > > index 2254038519e1..dec937b183a8 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > > @@ -9,6 +9,7 @@
> > > > #include <linux/soc/mediatek/mtk-cmdq.h>
> > > > #include <linux/soc/mediatek/mtk-mmsys.h>
> > > > #include <linux/soc/mediatek/mtk-mutex.h>
> > > > +#include "mtk_drm_ddp_comp.h"
> > > > #include "mtk_drm_plane.h"
> > > > #include "mtk_mdp_rdma.h"
> > > >
> > > > @@ -79,6 +80,7 @@ void mtk_ovl_clk_disable(struct device *dev);
> > > > void mtk_ovl_config(struct device *dev, unsigned int w,
> > > > unsigned int h, unsigned int vrefresh,
> > > > unsigned int bpc, struct cmdq_pkt
> > > > *cmdq_pkt);
> > > > +u64 mtk_ovl_get_sec_port(struct mtk_ddp_comp *comp, unsigned
> > > > int
> > > > idx);
> > > > int mtk_ovl_layer_check(struct device *dev, unsigned int idx,
> > > > struct mtk_plane_state *mtk_state);
> > > > void mtk_ovl_layer_config(struct device *dev, unsigned int
> > > > idx,
> > > > @@ -112,6 +114,7 @@ void mtk_ovl_adaptor_clk_disable(struct
> > > > device
> > > > *dev);
> > > > void mtk_ovl_adaptor_config(struct device *dev, unsigned int
> > > > w,
> > > > unsigned int h, unsigned int
> > > > vrefresh,
> > > > unsigned int bpc, struct cmdq_pkt
> > > > *cmdq_pkt);
> > > > +u64 mtk_ovl_adaptor_get_sec_port(struct mtk_ddp_comp *comp,
> > > > unsigned
> > > > int idx);
> > > > void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned
> > > > int
> > > > idx,
> > > > struct mtk_plane_state
> > > > *state,
> > > > struct cmdq_pkt *cmdq_pkt);
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > > index 2bffe4245466..76e832e4875a 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > > @@ -46,6 +46,7 @@
> > > > #define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data-
> > > > > addr +
> > > >
> > > > 0x20 * (n))
> > > > #define DISP_REG_OVL_HDR_ADDR(ovl, n) ((ovl)->data-
> > > > > addr + 0x20 * (n) + 0x04)
> > > >
> > > > #define DISP_REG_OVL_HDR_PITCH(ovl, n) ((ovl)->data-
> > > > > addr + 0x20 * (n) + 0x08)
> > > >
> > > > +#define DISP_REG_OVL_SECURE 0x0fc0
> > > >
> > > > #define GMC_THRESHOLD_BITS 16
> > > > #define GMC_THRESHOLD_HIGH ((1 << GMC_THRESHOLD_BITS) / 4)
> > > > @@ -126,8 +127,19 @@ struct mtk_disp_ovl {
> > > > const struct mtk_disp_ovl_data *data;
> > > > void (*vblank_cb)(void
> > > > *data);
> > > > void *vblank_cb_data;
> > > > + resource_size_t regs_pa;
> > > > };
> > > >
> > > > +u64 mtk_ovl_get_sec_port(struct mtk_ddp_comp *comp, unsigned
> > > > int
> > > > idx)
> > > > +{
> > > > + if (comp->id == DDP_COMPONENT_OVL0)
> > > > + return 1ULL << CMDQ_SEC_DISP_OVL0;
> > > > + else if (comp->id == DDP_COMPONENT_OVL1)
> > > > + return 1ULL << CMDQ_SEC_DISP_OVL1;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > static irqreturn_t mtk_disp_ovl_irq_handler(int irq, void
> > > > *dev_id)
> > > > {
> > > > struct mtk_disp_ovl *priv = dev_id;
> > > > @@ -449,8 +461,22 @@ void mtk_ovl_layer_config(struct device
> > > > *dev,
> > > > unsigned int idx,
> > > > DISP_REG_OVL_SRC_SIZE(idx));
> > > > mtk_ddp_write_relaxed(cmdq_pkt, offset, &ovl->cmdq_reg,
> > > > ovl-
> > > > > regs,
> > > >
> > > > DISP_REG_OVL_OFFSET(idx));
> > > > - mtk_ddp_write_relaxed(cmdq_pkt, addr, &ovl->cmdq_reg,
> > > > ovl-
> > > > > regs,
> > > >
> > > > - DISP_REG_OVL_ADDR(ovl, idx));
> > > > +
> > > > + if (state->pending.is_sec) {
> > > > + const struct drm_format_info *fmt_info =
> > > > drm_format_info(fmt);
> > > > + unsigned int buf_size = (pending->height - 1) *
> > > > pending->pitch +
> > > > + pending->width *
> > > > fmt_info-
> > > > > cpp[0];
> > > >
> > > > +
> > > > + mtk_ddp_write_mask(cmdq_pkt, BIT(idx), &ovl-
> > > > >cmdq_reg,
> > > > ovl->regs,
> > > > + DISP_REG_OVL_SECURE,
> > > > BIT(idx));
> > > > + mtk_ddp_sec_write(cmdq_pkt, ovl->regs_pa +
> > > > DISP_REG_OVL_ADDR(ovl, idx),
> > > > + pending->addr,
> > > > CMDQ_IWC_H_2_MVA, 0,
> > > > buf_size, 0);
> > >
> > > Why pass ovl->regs_pa into cmdq driver? cmdq just need subsys and
> > > offset.
> >
> > Yes, that's not necessary and I can refine that in the future.
> > Because
> > this change will also need to modify the code in CMDQ PTA, so I'll
> > fix
> > other issues in normal world first. Then I'll refine this place
> > after
> > a
> > few version.
>
> Add TODO information so that we would not forget this.
>
OK, I'll add TODO to the commit message.
> >
> > > In addition, why pass buf_size?
> > >
> >
> > This buf_size is the parameter for M4U PTA to verify the range of
> > secure address in the secure world.
>
> Why need this verification? In normal video playback, M4U does not
> verify the size. If hardware access out range of allocated buffer, it
> would cause M4U to have translation fault. I think secure video could
> also have translation fault so the size verification is not
> necessary.
>
I've checked this with M4U owner. It's not only used to verification,
but also used to tell m4U TA how much buffer size it needs to map to
iova in secure world. So it's a necessary information.
Regards,
Jason-JH.Lin
> Regards,
> CK
>
>
> >
> > Regards,
> > Jason-JH.Lin
> >
> > > Regards,
> > > CK
> > >
