Hi Neil,
On Tue, May 26, 2020 at 02:35:19PM +0200, Neil Armstrong wrote:
On 26/05/2020 03:15, Laurent Pinchart wrote:
Implement the drm_bridge_funcs .detect() and .get_edid() operations, and call drm_bridge_hpd_notify() notify to report HPD. This provides the necessary API to support disabling connector creation, do so by accepting DRM_BRIDGE_ATTACH_NO_CONNECTOR in dw_hdmi_bridge_attach().
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 104 +++++++++++++++------- 1 file changed, 74 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index b69c14b9de62..6148a022569a 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2323,15 +2323,8 @@ static void dw_hdmi_update_phy_mask(struct dw_hdmi *hdmi) hdmi->rxsense); }
-/* -----------------------------------------------------------------------------
- DRM Connector Operations
- */
-static enum drm_connector_status -dw_hdmi_connector_detect(struct drm_connector *connector, bool force) +static enum drm_connector_status dw_hdmi_detect(struct dw_hdmi *hdmi) {
struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
connector);enum drm_connector_status result;
mutex_lock(&hdmi->mutex);
@@ -2354,31 +2347,57 @@ dw_hdmi_connector_detect(struct drm_connector *connector, bool force) return result; }
-static int dw_hdmi_connector_get_modes(struct drm_connector *connector) +static struct edid *dw_hdmi_get_edid(struct dw_hdmi *hdmi,
struct drm_connector *connector){
struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
connector);struct edid *edid;
int ret = 0;
if (!hdmi->ddc)
return 0;
return NULL;edid = drm_get_edid(connector, hdmi->ddc);
- if (edid) {
dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",edid->width_cm, edid->height_cm);hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);hdmi->sink_has_audio = drm_detect_monitor_audio(edid);drm_connector_update_edid_property(connector, edid);cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid);ret = drm_add_edid_modes(connector, edid);kfree(edid);- } else {
if (!edid) { dev_dbg(hdmi->dev, "failed to get edid\n");
return NULL;}
dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
edid->width_cm, edid->height_cm);hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
return edid;
+}
+/* -----------------------------------------------------------------------------
- DRM Connector Operations
- */
+static enum drm_connector_status +dw_hdmi_connector_detect(struct drm_connector *connector, bool force) +{
- struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
connector);- return dw_hdmi_detect(hdmi);
+}
+static int dw_hdmi_connector_get_modes(struct drm_connector *connector) +{
- struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
connector);- struct edid *edid;
- int ret;
- edid = dw_hdmi_get_edid(hdmi, connector);
- if (!edid)
return 0;- drm_connector_update_edid_property(connector, edid);
- cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid);
- ret = drm_add_edid_modes(connector, edid);
- kfree(edid);
- return ret;
}
@@ -2777,10 +2796,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge, { struct dw_hdmi *hdmi = bridge->driver_private;
- if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
DRM_ERROR("Fix bridge driver to make connector optional!");return -EINVAL;- }
if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
return 0;return dw_hdmi_connector_create(hdmi);
} @@ -2860,6 +2877,21 @@ static void dw_hdmi_bridge_atomic_enable(struct drm_bridge *bridge, mutex_unlock(&hdmi->mutex); }
+static enum drm_connector_status dw_hdmi_bridge_detect(struct drm_bridge *bridge) +{
- struct dw_hdmi *hdmi = bridge->driver_private;
- return dw_hdmi_detect(hdmi);
+}
+static struct edid *dw_hdmi_bridge_get_edid(struct drm_bridge *bridge,
struct drm_connector *connector)+{
- struct dw_hdmi *hdmi = bridge->driver_private;
- return dw_hdmi_get_edid(hdmi, connector);
+}
static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = { .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, @@ -2873,6 +2905,8 @@ static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = { .atomic_disable = dw_hdmi_bridge_atomic_disable, .mode_set = dw_hdmi_bridge_mode_set, .mode_valid = dw_hdmi_bridge_mode_valid,
- .detect = dw_hdmi_bridge_detect,
- .get_edid = dw_hdmi_bridge_get_edid,
};
/* ----------------------------------------------------------------------------- @@ -2988,10 +3022,18 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) }
if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
enum drm_connector_status status = phy_int_pol & HDMI_PHY_HPD? connector_status_connected: connector_status_disconnected;- dev_dbg(hdmi->dev, "EVENT=%s\n",
phy_int_pol & HDMI_PHY_HPD ? "plugin" : "plugout");if (hdmi->bridge.dev)
status == connector_status_connected ?"plugin" : "plugout");if (hdmi->bridge.dev) { drm_helper_hpd_irq_event(hdmi->bridge.dev);drm_bridge_hpd_notify(&hdmi->bridge, status);I suspect I will also need to add drm_bridge_hpd_notify() in meson_dw_hdmi.c in dw_hdmi_top_thread_irq() for HPD event, right ?
If you want to support DRM_BRIDGE_ATTACH_NO_CONNECTOR (and I think you should :-)), yes.
}}
hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);
@@ -3337,6 +3379,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
hdmi->bridge.driver_private = hdmi; hdmi->bridge.funcs = &dw_hdmi_bridge_funcs;
- hdmi->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID
| DRM_BRIDGE_OP_HPD;same here for meson_dw_hdmi ? could I also assume we could disable the dw_hdmi bridge & hpd ops when using with meson_dw_hdmi and implement these in meson_dw_hdmi ?
I've only noticed now that meson_dw_hdmi has its own bridge. Could you briefly explain how all that works ?
#ifdef CONFIG_OF hdmi->bridge.of_node = pdev->dev.of_node; #endif
Anyway
Reviewed-by: Neil Armstrong narmstrong@baylibre.com