Sorry, I fix a link error below. On Mon, 2021-09-06 at 00:23 +0800, houlong wei wrote: > Hi Ezequiel, > > Thank you for your attention to this series of patches. I answer > partial of your questions below. > Regards, > Houlong > > On Sat, 2021-09-04 at 20:34 +0800, Ezequiel Garcia wrote: > > Hi Eizan, > > > > Sorry for seeing this series so late. > > > > On Wed, 25 Aug 2021 at 03:35, Eizan Miyamoto <eizan@xxxxxxxxxxxx> > > wrote: > > > > > > Broadly, this patch (1) adds a driver for various MTK MDP > > > components to > > > go alongside the main MTK MDP driver, and (2) hooks them all > > > together > > > using the component framework. > > > > > > (1) Up until now, the MTK MDP driver controls 8 devices in the > > > device > > > tree on its own. When running tests for the hardware video > > > decoder, > > > we > > > found that the iommus and LARBs were not being properly > > > configured. > > > > Why were not being properly configured? What was the problem? > > Why not fixing that instead? > > > > Does this mean the driver is currently broken and unusable? > > This series of patches are supplements to another series, please > refer > to > https://patchwork.kernel.org/project/linux-mediatek/list/?series=515129c Fix the link address: https://patchwork.kernel.org/project/linux-mediatek/list/?series=523771 > , which add device link between the mtk-iommu consumer and the mtk- > larb > devices. Without that series of patches, the mtk-mdp driver can work > well so far. > But with that series, it seems the device link only can be > established > for the device which is registered as a platform driver. That's why > Eizan adds this series of patches to make all mdp components to be > registered as platform drivers. > > > > > > To > > > configure them, a driver for each be added to mtk_mdp_comp so > > > that > > > mtk_iommu_add_device() can (eventually) be called from > > > dma_configure() > > > inside really_probe(). > > > > > > (2) The integration into the component framework allows us to > > > defer > > > the > > > registration with the v4l2 subsystem until all the MDP-related > > > devices > > > have been probed, so that the relevant device node does not > > > become > > > available until initialization of all the components is complete. > > > > > > Some notes about how the component framework has been integrated: > > > > > > - The driver for the rdma0 component serves double duty as the > > > "master" > > > (aggregate) driver as well as a component driver. This is a > > > non- > > > ideal > > > compromise until a better solution is developed. This device is > > > differentiated from the rest by checking for a "mediatek,vpu" > > > property > > > in the device node. > > > > > > > As I have stated in Yunfei, I am not convinced you need an async > > framework > > at all. It seems all these devices could have been linked together > > in the device tree, and then have a master device to tie them. > > > > I.e. something like > > > > mdp { > > mdp_rdma0 { > > } > > mdp_rsz0 { > > } > > mdp_rsz1 { > > } > > } > > > > The commit message of the patch below explains that " If the mdp_* > nodes are under an mdp sub-node, their corresponding platform device > does not automatically get its iommu assigned properly." > Please refer to > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.14.1&id=8127881f741dbbf9a1da9e9bc59133820160b217 > > > All this async games seem like making the driver really obfuscated, > > which will mean harder to debug and maintain. > > I am not sure we want that burden. > > > > Even if we are all fully convinced that you absolutely need > > an async framework, then what's wrong with v4l2-async? > > > > I would start by addressing what is wrong with the IOMMUs > > in the current design. > > > > Thanks, > > Ezequiel > > > > > - The list of mdp components remains hard-coded as > > > mtk_mdp_comp_dt_ids[] > > > in mtk_mdp_core.c, and as mtk_mdp_comp_driver_dt_match[] in > > > mtk_mdp_comp.c. This unfortunate duplication of information is > > > addressed in a following patch in this series. > > > > > > - The component driver calls component_add() for each device that > > > is > > > probed. > > > > > > - In mtk_mdp_probe (the "master" device), we scan the device tree > > > for > > > any matching nodes against mtk_mdp_comp_dt_ids, and add > > > component > > > matches for them. The match criteria is a matching device node > > > pointer. > > > > > > - When the set of components devices that have been probed > > > corresponds > > > with the list that is generated by the "master", the callback > > > to > > > mtk_mdp_master_bind() is made, which then calls the component > > > bind > > > functions. > > > > > > - Inside mtk_mdp_master_bind(), once all the component bind > > > functions > > > have been called, we can then register our device to the v4l2 > > > subsystem. > > > > > > - The call to pm_runtime_enable() in the master device is called > > > after > > > all the components have been registered by their bind() > > > functions > > > called by mtk_mtp_master_bind(). As a result, the list of > > > components > > > will not change while power management callbacks > > > mtk_mdp_suspend()/ > > > resume() are accessing the list of components. > > > > > > Signed-off-by: Eizan Miyamoto <eizan@xxxxxxxxxxxx> > > > Reviewed-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx > > > > > > > Reviewed-by: Houlong Wei <houlong.wei@xxxxxxxxxxxx> > > > --- > > > > >
