| Message ID | 20230802144802.751-5-jason-jh.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add dynamic connector selection mechanism | expand |
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@mediatek.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,
Hi Eugen, Thanks for the reviews. On Thu, 2023-08-03 at 16:27 +0300, Eugen Hristev wrote: > 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. > OK, I'll move the second part to [PATCH v8 6/8]. > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.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 > OK, I'll move it between `unsigned int id` and `const struct mtk_ddp_comp_funcs *funcs`. > > }; > > > > 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 ? > Because it's used in mtk_drm_crtc_create() in [PATCH v8 6/8]. I'll move this part into [PATCH v8 6/8]. Regards, Jason-JH.Lin > > + > > 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, > >
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; }; 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); +} + 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,
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. Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> --- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 8 ++++++++ 1 file changed, 8 insertions(+)