| Message ID | 20231016064346.31451-1-yunfei.dong@mediatek.com |
|---|---|
| State | Accepted |
| Commit | 8830bf13af87f69755df0ffca6d0169ed45ac1ee |
| Headers | show |
| Series | [1/7] media: mediatek: vcodec: Getting the chip name of each platform | expand |
Il 16/10/23 08:43, Yunfei Dong ha scritto: > Getting the chip name of each platform according to the device > compatible to set different parameter. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Hi Sebastian, Thanks for your advice. Send the patch v2 to fix the below items: Fix the commit message for some patches. Fix the level of mt8195 and mt8192. Best Regards, Yunfei Dong On Sat, 2023-10-21 at 10:30 +0200, Sebastian Fricke wrote: > Hey Yunfei, > > Thanks for your patches! > > Could you provide a cover-letter for the next version please? > This will help to get a good context of why we need these changes and > to > store the changelog in a helpful manner. > Thanks. > > On 16.10.2023 14:43, Yunfei Dong wrote: > > Getting the chip name of each platform according to the device > > compatible to set different parameter. > > I would reword this commit description slightly, basically what you > change is that you store the chip name in context permanently and > that > you utilize a enum to be more descriptive. > > So how about: > > """ > Store the name of the chip in the context of the driver in order to > be > able to choose the correct configuration values for the different > codecs. > Use a enum value instead of an integer to store a more descriptive > name. > """ > > A few more comments below. > > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > --- > > .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 24 +--------------- > > - > > .../vcodec/decoder/mtk_vcodec_dec_drv.c | 26 > > +++++++++++++++++++ > > .../vcodec/decoder/mtk_vcodec_dec_drv.h | 17 ++++++++++++ > > 3 files changed, 44 insertions(+), 23 deletions(-) > > > > diff --git > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c > > index 91ed576d6821..ba742f0e391d 100644 > > --- > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c > > +++ > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c > > @@ -208,36 +208,14 @@ static int vidioc_vdec_dqbuf(struct file > > *file, void *priv, > > return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf); > > } > > > > -static int mtk_vcodec_dec_get_chip_name(void *priv) > > -{ > > - struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv); > > - struct device *dev = &ctx->dev->plat_dev->dev; > > - > > - if (of_device_is_compatible(dev->of_node, "mediatek,mt8173- > > vcodec-dec")) > > - return 8173; > > - else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8183-vcodec-dec")) > > - return 8183; > > - else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8192-vcodec-dec")) > > - return 8192; > > - else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8195-vcodec-dec")) > > - return 8195; > > - else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8186-vcodec-dec")) > > - return 8186; > > - else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8188-vcodec-dec")) > > - return 8188; > > - else > > - return 8173; > > -} > > - > > static int vidioc_vdec_querycap(struct file *file, void *priv, > > struct v4l2_capability *cap) > > { > > struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv); > > struct device *dev = &ctx->dev->plat_dev->dev; > > - int platform_name = mtk_vcodec_dec_get_chip_name(priv); > > > > strscpy(cap->driver, dev->driver->name, sizeof(cap->driver)); > > - snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", > > platform_name); > > + snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", > > ctx->dev->chip_name); > > > > return 0; > > } > > diff --git > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv > > .c > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv > > .c > > index 0a89ce452ac3..f47c98faf068 100644 > > --- > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv > > .c > > +++ > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv > > .c > > @@ -326,6 +326,26 @@ static const struct v4l2_file_operations > > mtk_vcodec_fops = { > > .mmap = v4l2_m2m_fop_mmap, > > }; > > > > +static void mtk_vcodec_dec_get_chip_name(struct mtk_vcodec_dec_dev > > *vdec_dev) > > +{ > > + struct device *dev = &vdec_dev->plat_dev->dev; > > + > > + if (of_device_is_compatible(dev->of_node, "mediatek,mt8173- > > vcodec-dec")) > > + vdec_dev->chip_name = MTK_VDEC_MT8173; > > + else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8183-vcodec-dec")) > > + vdec_dev->chip_name = MTK_VDEC_MT8183; > > + else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8192-vcodec-dec")) > > + vdec_dev->chip_name = MTK_VDEC_MT8192; > > + else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8195-vcodec-dec")) > > + vdec_dev->chip_name = MTK_VDEC_MT8195; > > + else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8186-vcodec-dec")) > > + vdec_dev->chip_name = MTK_VDEC_MT8186; > > + else if (of_device_is_compatible(dev->of_node, > > "mediatek,mt8188-vcodec-dec")) > > + vdec_dev->chip_name = MTK_VDEC_MT8188; > > + else > > + vdec_dev->chip_name = MTK_VDEC_INVAL; > > +} > > + > > static int mtk_vcodec_probe(struct platform_device *pdev) > > { > > struct mtk_vcodec_dec_dev *dev; > > @@ -341,6 +361,12 @@ static int mtk_vcodec_probe(struct > > platform_device *pdev) > > INIT_LIST_HEAD(&dev->ctx_list); > > dev->plat_dev = pdev; > > > > + mtk_vcodec_dec_get_chip_name(dev); > > + if (dev->chip_name == MTK_VDEC_INVAL) { > > + dev_err(&pdev->dev, "Failed to get decoder chip name"); > > + return -EINVAL; > > + } > > + > > dev->vdec_pdata = of_device_get_match_data(&pdev->dev); > > if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu", > > &rproc_phandle)) { > > diff --git > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv > > .h > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv > > .h > > index 7e36b2c69b7d..8f228ba9aa47 100644 > > --- > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv > > .h > > +++ > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv > > .h > > @@ -18,6 +18,19 @@ > > #define IS_VDEC_LAT_ARCH(hw_arch) ((hw_arch) >= > > MTK_VDEC_LAT_SINGLE_CORE) > > #define IS_VDEC_INNER_RACING(capability) ((capability) & > > MTK_VCODEC_INNER_RACING) > > > > +/* > > + * enum mtk_vcodec_dec_chip_name - Structure used to separate > > different platform > > + */ > > I don't feel like this comment is terribly helpful because it is > pretty > clear what the enum is about, I would just drop it. > > > +enum mtk_vcodec_dec_chip_name { > > + MTK_VDEC_INVAL = 0, > > + MTK_VDEC_MT8173 = 8173, > > + MTK_VDEC_MT8183 = 8183, > > + MTK_VDEC_MT8186 = 8186, > > + MTK_VDEC_MT8188 = 8188, > > + MTK_VDEC_MT8192 = 8192, > > + MTK_VDEC_MT8195 = 8195, > > +}; > > + > > /* > > * enum mtk_vdec_format_types - Structure used to get supported > > * format types according to decoder capability > > @@ -249,6 +262,8 @@ struct mtk_vcodec_dec_ctx { > > * @vdec_racing_info: record register value > > * @dec_racing_info_mutex: mutex lock used for inner racing mode > > * @dbgfs: debug log related information > > + * > > + * @chip_name: the chip name used to separate different platform > > I wouldn't repeat chip name in the description and specify more > concretely why we need to separate the platforms. > > My suggestion: > > * @chip_name: used to distinguish platforms and select the correct > codec configuration values. > > > */ > > struct mtk_vcodec_dec_dev { > > struct v4l2_device v4l2_dev; > > @@ -289,6 +304,8 @@ struct mtk_vcodec_dec_dev { > > /* Protects access to vdec_racing_info data */ > > struct mutex dec_racing_info_mutex; > > struct mtk_vcodec_dbgfs dbgfs; > > + > > + enum mtk_vcodec_dec_chip_name chip_name; > > }; > > > > static inline struct mtk_vcodec_dec_ctx *fh_to_dec_ctx(struct > > v4l2_fh *fh) > > Besides those small wording choices, the patch looks good. > > So with these issues resolved: > > Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com> > > Regards, > Sebastian > > -- > > 2.18.0 > >
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c index 91ed576d6821..ba742f0e391d 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c @@ -208,36 +208,14 @@ static int vidioc_vdec_dqbuf(struct file *file, void *priv, return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf); } -static int mtk_vcodec_dec_get_chip_name(void *priv) -{ - struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv); - struct device *dev = &ctx->dev->plat_dev->dev; - - if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec")) - return 8173; - else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec")) - return 8183; - else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec")) - return 8192; - else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec")) - return 8195; - else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec")) - return 8186; - else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec")) - return 8188; - else - return 8173; -} - static int vidioc_vdec_querycap(struct file *file, void *priv, struct v4l2_capability *cap) { struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv); struct device *dev = &ctx->dev->plat_dev->dev; - int platform_name = mtk_vcodec_dec_get_chip_name(priv); strscpy(cap->driver, dev->driver->name, sizeof(cap->driver)); - snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", platform_name); + snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", ctx->dev->chip_name); return 0; } diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c index 0a89ce452ac3..f47c98faf068 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c @@ -326,6 +326,26 @@ static const struct v4l2_file_operations mtk_vcodec_fops = { .mmap = v4l2_m2m_fop_mmap, }; +static void mtk_vcodec_dec_get_chip_name(struct mtk_vcodec_dec_dev *vdec_dev) +{ + struct device *dev = &vdec_dev->plat_dev->dev; + + if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec")) + vdec_dev->chip_name = MTK_VDEC_MT8173; + else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec")) + vdec_dev->chip_name = MTK_VDEC_MT8183; + else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec")) + vdec_dev->chip_name = MTK_VDEC_MT8192; + else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec")) + vdec_dev->chip_name = MTK_VDEC_MT8195; + else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec")) + vdec_dev->chip_name = MTK_VDEC_MT8186; + else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec")) + vdec_dev->chip_name = MTK_VDEC_MT8188; + else + vdec_dev->chip_name = MTK_VDEC_INVAL; +} + static int mtk_vcodec_probe(struct platform_device *pdev) { struct mtk_vcodec_dec_dev *dev; @@ -341,6 +361,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev) INIT_LIST_HEAD(&dev->ctx_list); dev->plat_dev = pdev; + mtk_vcodec_dec_get_chip_name(dev); + if (dev->chip_name == MTK_VDEC_INVAL) { + dev_err(&pdev->dev, "Failed to get decoder chip name"); + return -EINVAL; + } + dev->vdec_pdata = of_device_get_match_data(&pdev->dev); if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu", &rproc_phandle)) { diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h index 7e36b2c69b7d..8f228ba9aa47 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h @@ -18,6 +18,19 @@ #define IS_VDEC_LAT_ARCH(hw_arch) ((hw_arch) >= MTK_VDEC_LAT_SINGLE_CORE) #define IS_VDEC_INNER_RACING(capability) ((capability) & MTK_VCODEC_INNER_RACING) +/* + * enum mtk_vcodec_dec_chip_name - Structure used to separate different platform + */ +enum mtk_vcodec_dec_chip_name { + MTK_VDEC_INVAL = 0, + MTK_VDEC_MT8173 = 8173, + MTK_VDEC_MT8183 = 8183, + MTK_VDEC_MT8186 = 8186, + MTK_VDEC_MT8188 = 8188, + MTK_VDEC_MT8192 = 8192, + MTK_VDEC_MT8195 = 8195, +}; + /* * enum mtk_vdec_format_types - Structure used to get supported * format types according to decoder capability @@ -249,6 +262,8 @@ struct mtk_vcodec_dec_ctx { * @vdec_racing_info: record register value * @dec_racing_info_mutex: mutex lock used for inner racing mode * @dbgfs: debug log related information + * + * @chip_name: the chip name used to separate different platform */ struct mtk_vcodec_dec_dev { struct v4l2_device v4l2_dev; @@ -289,6 +304,8 @@ struct mtk_vcodec_dec_dev { /* Protects access to vdec_racing_info data */ struct mutex dec_racing_info_mutex; struct mtk_vcodec_dbgfs dbgfs; + + enum mtk_vcodec_dec_chip_name chip_name; }; static inline struct mtk_vcodec_dec_ctx *fh_to_dec_ctx(struct v4l2_fh *fh)
Getting the chip name of each platform according to the device compatible to set different parameter. Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> --- .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 24 +---------------- .../vcodec/decoder/mtk_vcodec_dec_drv.c | 26 +++++++++++++++++++ .../vcodec/decoder/mtk_vcodec_dec_drv.h | 17 ++++++++++++ 3 files changed, 44 insertions(+), 23 deletions(-)