Re: [PATCH v3 09/11] drm/mediatek: Add secure flow support to mediatek-drm

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

Hi CK,

Thanks for the reviews.

On Tue, 2023-12-26 at 05:43 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
> 
> On Sun, 2023-12-24 at 02:29 +0800, Jason-JH.Lin wrote:
> > To add secure flow support for mediatek-drm, each crtc have to
> > create a secure cmdq mailbox channel. Then cmdq packets with
> > display HW configuration will be sent to secure cmdq mailbox
> > channel
> > and configured in the secure world.
> > 
> > Each crtc have to use secure cmdq interface to configure some
> > secure
> > settings for display HW before sending cmdq packets to secure cmdq
> > mailbox channel.
> > 
> > If any of fb get from current drm_atomic_state is secure, then crtc
> > will switch to the secure flow to configure display HW.
> > If all fbs are not secure in current drm_atomic_state, then crtc
> > will
> > switch to the normal flow.
> > 
> > TODO:
> > 1. Remove get sec larb port interface in ddp_comp, ovl and
> > ovl_adaptor.
> > 2. Verify instruction for enabling/disabling dapc and larb port in
> > TEE
> >    drop the sec_engine flags in normal world.
> > 
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > 
> 
> [snip]
> 
> > @@ -1091,14 +1292,63 @@ int mtk_drm_crtc_create(struct drm_device
> > *drm_dev,
> >  			if (ret) {
> >  				dev_dbg(dev, "mtk_crtc %d failed to
> > create cmdq packet\n",
> >  					drm_crtc_index(&mtk_crtc-
> > > base));
> > 
> > -				mbox_free_channel(mtk_crtc-
> > > cmdq_client.chan);
> > 
> > -				mtk_crtc->cmdq_client.chan = NULL;
> > +				goto cmdq_err;
> >  			}
> >  		}
> >  
> >  		/* for sending blocking cmd in crtc disable */
> >  		init_waitqueue_head(&mtk_crtc->cb_blocking_queue);
> >  	}
> > +
> > +	mtk_crtc->sec_cmdq_client.client.dev = mtk_crtc->mmsys_dev;
> > +	mtk_crtc->sec_cmdq_client.client.tx_block = false;
> > +	mtk_crtc->sec_cmdq_client.client.knows_txdone = true;
> > +	mtk_crtc->sec_cmdq_client.client.rx_callback = ddp_cmdq_cb;
> > +	mtk_crtc->sec_cmdq_client.chan =
> > +			mbox_request_channel(&mtk_crtc-
> > > sec_cmdq_client.client, i + 1);
> > 
> > +	if (IS_ERR(mtk_crtc->sec_cmdq_client.chan)) {
> > +		dev_err(dev, "mtk_crtc %d failed to create sec mailbox
> > client\n",
> > +			drm_crtc_index(&mtk_crtc->base));
> > +		mtk_crtc->sec_cmdq_client.chan = NULL;
> > +	}
> > +
> > +	if (mtk_crtc->sec_cmdq_client.chan) {
> 
> I would like use secure channel to replace normal channel. It means
> that no extra channel is required and change the original normal
> channel to secure channel. The secure channel could process both
> normal
> buffer and secure buffer, so you need not to switch the channel.
> 
> Regards,
> CK

It sounds quite reasonable!

If the platform or project support OPTEE, we can default use secure
channel to handle both normal and secure buffers. 
I will try to to refine this and make sure it won't cause latency issue
on OPTEE transaction frequently.

Regards,
Jason-JH.Lin