Re: [PATCH v13 5/9] drm/synopsys+ingenic: repair hot plug detection
Hi Paul,
> Am 09.02.2022 um 13:01 schrieb Paul Cercueil :
>
> Hi Nikolaus,
>
> Le mer., févr. 2 2022 at 17:31:19 +0100, H. Nikolaus Schaller
> 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
>> ---
>> 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
Re: [PATCH v13 5/9] drm/synopsys+ingenic: repair hot plug detection
Hi Nikolaus,
Le mer., févr. 2 2022 at 17:31:19 +0100, H. Nikolaus Schaller
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
---
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
[PATCH v13 5/9] drm/synopsys+ingenic: repair hot plug detection
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
---
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);
+
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
