| Message ID | 08fb9549042d35c1904fd977e68aa52f74f755b0.1643819482.git.hns@goldelico.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: JZ4780 and CI20 HDMI | expand |
Hi Nikolaus, Le mer., févr. 2 2022 at 17:31:19 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit : > so that it calls drm_kms_helper_hotplug_event(). > > We need to set .poll_enabled but that struct component > can only be accessed in the core code. Hence we add a public > setter function. > > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++ > drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 2 ++ > include/drm/bridge/dw_hdmi.h | 1 + > 3 files changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 54d8fdad395f5..52e7cd2e020d3 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -3216,6 +3216,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi > *hdmi) > return 0; > } > > +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable) > +{ > + if (hdmi->bridge.dev) > + hdmi->bridge.dev->mode_config.poll_enabled = enable; > + else > + dev_warn(hdmi->dev, "no hdmi->bridge.dev"); > +} > +EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll); > + > struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev, > const struct dw_hdmi_plat_data *plat_data) > { > diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c > b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c > index 34e986dd606cf..90547a28dc5c7 100644 > --- a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c > +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c > @@ -55,6 +55,8 @@ ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, > void *data, > if (mode->clock > 216000) > return MODE_CLOCK_HIGH; > > + dw_hdmi_enable_poll(hdmi, true); > + It would be a better idea to move this patch before the patch that creates ingenic-dw-hdmi.c. Then you wouldn't have to patch a file that was just introduced. As for the patch itself, I guess it's fine, but is that really needed? My understanding is that it's the hdmi-connector's job to poll. Cheers, -Paul > return MODE_OK; > } > > diff --git a/include/drm/bridge/dw_hdmi.h > b/include/drm/bridge/dw_hdmi.h > index 2a1f85f9a8a3f..963960794b40e 100644 > --- a/include/drm/bridge/dw_hdmi.h > +++ b/include/drm/bridge/dw_hdmi.h > @@ -196,5 +196,6 @@ enum drm_connector_status > dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi, > void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data, > bool force, bool disabled, bool rxsense); > void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data); > +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable); > > #endif /* __IMX_HDMI_H__ */ > -- > 2.33.0 >
Hi Paul, > Am 09.02.2022 um 13:01 schrieb Paul Cercueil <paul@crapouillou.net>: > > Hi Nikolaus, > > Le mer., févr. 2 2022 at 17:31:19 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit : >> so that it calls drm_kms_helper_hotplug_event(). >> We need to set .poll_enabled but that struct component >> can only be accessed in the core code. Hence we add a public >> setter function. >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >> --- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++ >> drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 2 ++ >> include/drm/bridge/dw_hdmi.h | 1 + >> 3 files changed, 12 insertions(+) >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index 54d8fdad395f5..52e7cd2e020d3 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -3216,6 +3216,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi) >> return 0; >> } >> +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable) >> +{ >> + if (hdmi->bridge.dev) >> + hdmi->bridge.dev->mode_config.poll_enabled = enable; >> + else >> + dev_warn(hdmi->dev, "no hdmi->bridge.dev"); >> +} >> +EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll); >> + >> struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev, >> const struct dw_hdmi_plat_data *plat_data) >> { >> diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c >> index 34e986dd606cf..90547a28dc5c7 100644 >> --- a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c >> +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c >> @@ -55,6 +55,8 @@ ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data, >> if (mode->clock > 216000) >> return MODE_CLOCK_HIGH; >> + dw_hdmi_enable_poll(hdmi, true); >> + > > It would be a better idea to move this patch before the patch that creates ingenic-dw-hdmi.c. Then you wouldn't have to patch a file that was just introduced. The main reason to have a separate patch was that I was not sure what is already merged somewhere and what is not. And fixing something which is not yet introduced makes it quite difficult to explain, why it is needed at all... So I would prefer to leave it as is until more comments arrive. > > As for the patch itself, I guess it's fine, but is that really needed? My understanding is that it's the hdmi-connector's job to poll. The hardware gpio that we can define for the hdmi-connector seems not to be available on all CI20 boards. Hence we must trigger (enable) the poll logic of the dw-hdmi bridge from the SoC specialization. This seems to be best done in the ingenic-dw-hdmi driver. Unless someone has a better idea how the dw-hdmi driver could find out that it should poll if a connector is defined. The base driver seems as if it has been developed long ago without connectors and bridge chains in mind. Hence we are retrofitting fixes for changes introduced outside the drivers. BR, Nikolaus
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 54d8fdad395f5..52e7cd2e020d3 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -3216,6 +3216,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi) return 0; } +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable) +{ + if (hdmi->bridge.dev) + hdmi->bridge.dev->mode_config.poll_enabled = enable; + else + dev_warn(hdmi->dev, "no hdmi->bridge.dev"); +} +EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll); + struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev, const struct dw_hdmi_plat_data *plat_data) { diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c index 34e986dd606cf..90547a28dc5c7 100644 --- a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c @@ -55,6 +55,8 @@ ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data, if (mode->clock > 216000) return MODE_CLOCK_HIGH; + dw_hdmi_enable_poll(hdmi, true); + return MODE_OK; } diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h index 2a1f85f9a8a3f..963960794b40e 100644 --- a/include/drm/bridge/dw_hdmi.h +++ b/include/drm/bridge/dw_hdmi.h @@ -196,5 +196,6 @@ enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi, void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data, bool force, bool disabled, bool rxsense); void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data); +void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable); #endif /* __IMX_HDMI_H__ */
so that it calls drm_kms_helper_hotplug_event(). We need to set .poll_enabled but that struct component can only be accessed in the core code. Hence we add a public setter function. Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++ drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 2 ++ include/drm/bridge/dw_hdmi.h | 1 + 3 files changed, 12 insertions(+)