[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <a0b5ab0a-4c2e-d14c-508c-09a981d7087b@collabora.com>
Date: Thu, 3 Aug 2023 16:27:16 +0300
From: Eugen Hristev <eugen.hristev@...labora.com>
To: "Jason-JH.Lin" <jason-jh.lin@...iatek.com>,
Chun-Kuang Hu <chunkuang.hu@...nel.org>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
Cc: Matthias Brugger <matthias.bgg@...il.com>,
Jason-ch Chen <jason-ch.chen@...iatek.com>,
Johnson Wang <johnson.wang@...iatek.com>,
Singo Chang <singo.chang@...iatek.com>,
Nancy Lin <nancy.lin@...iatek.com>,
Shawn Sung <shawn.sung@...iatek.com>,
dri-devel@...ts.freedesktop.org,
linux-mediatek@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v8 4/8] drm/mediatek: Add encoder_index function to
mtk_ddp_comp_funcs
On 8/2/23 17:47, Jason-JH.Lin wrote:
> 1. Add encoder_index function to mtk_ddp_comp_funcs to support dynamic
> connector selection for some ddp_comp who has encoder_index.
> 2. Add mtk_ddp_comp_encoder_index_set function to set encoder_index to
> each comp.
>
Usually a commit that does two things in a list is supposed to be two
actual commits.
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index febcaeef16a1..8428baca70f4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -80,6 +80,7 @@ struct mtk_ddp_comp_funcs {
> void (*disconnect)(struct device *dev, struct device *mmsys_dev, unsigned int next);
> void (*add)(struct device *dev, struct mtk_mutex *mutex);
> void (*remove)(struct device *dev, struct mtk_mutex *mutex);
> + unsigned int (*encoder_index)(struct device *dev);
> };
>
> struct mtk_ddp_comp {
> @@ -87,6 +88,7 @@ struct mtk_ddp_comp {
> int irq;
> unsigned int id;
> const struct mtk_ddp_comp_funcs *funcs;
> + unsigned int encoder_index;
For better alignment I would suggest variables to be declared together
and pointers afterwards, not mixed up
> };
>
> static inline int mtk_ddp_comp_clk_enable(struct mtk_ddp_comp *comp)
> @@ -275,6 +277,12 @@ static inline bool mtk_ddp_comp_disconnect(struct mtk_ddp_comp *comp, struct dev
> return false;
> }
>
> +static inline void mtk_ddp_comp_encoder_index_set(struct mtk_ddp_comp *comp)
> +{
> + if (comp->funcs && comp->funcs->encoder_index)
> + comp->encoder_index = comp->funcs->encoder_index(comp->dev);
> +}
it's also a bit strange that you added a function that is not used
anywhere. Don't you get like a compiler warning for it ?
> +
> int mtk_ddp_comp_get_id(struct device_node *node,
> enum mtk_ddp_comp_type comp_type);
> unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
Powered by blists - more mailing lists