From: Hans Verkuil hans.verkuil@cisco.com
Now that the DisplayPort CEC-Tunneling-over-AUX drm+i915 support has been merged in the mainline kernel it is time to roll this out to nouveau and amdgpu as well.
I combined both in the same patch series since both depend on the same first patch, the comments in this cover letter apply to both and the implementation is also very similar (and simple).
As mentioned, the first patch is required for this: it adds checks that the drm_dp_cec functions are called with a working aux implementation. These checks weren't necessary for the i915, but nouveau and amdgpu require them.
The next two patches update a comment in drm_dp_cec.c and fix a bug in sideband AUX handling that I found while researching CEC Tunneling over an MST hub. It's there to prevent others from stumbling over the same bug in the future.
The fourth patch adds support for CEC to the nouveau driver.
The last patch adds support for CEC to the amdgpu driver. However, there appear to be two classes of amdgpu hardware: as a discrete GPU or integrated. I only have a discrete GPU, so I can't test the integrated GPU support and I only implemented this for the discrete GPU case.
If someone has the integrated GPU and wants to get this working and is willing to do some testing, then please contact me. It shouldn't be difficult. You will likely have to buy a working DP-to-HDMI adapter. See https://hverkuil.home.xs4all.nl/cec-status.txt for a (sadly very short) list of working adapters.
Note that I may be completely off-base regarding what atombios_dp.c does, it's the first time I ever looked at amdgpu code.
Two notes on CEC-Tunneling-over-AUX:
1) You need to be very careful about which USB-C/DP/mini-DP to HDMI adapters you buy. Only a few support this feature correctly today. Known chipsets that support this are Parade PS175 & PS176 and MegaChips 2900. Unfortunately almost all Parade-based adapters do not hook up the HDMI CEC pin to the chip, making them useless for this. The Parade reference design was only recently changed to hook up this pin, so perhaps this situation will change for new Parade-based adapters.
Adapters based on the new MegaChips 2900 fare much better: it appears that their reference design *does* hook up the CEC pin. Club3D has adapters using this device for USB-C, DP and mini-DP to HDMI, and they all work fine.
If anyone finds other adapters that work besides those I list in https://hverkuil.home.xs4all.nl/cec-status.txt, please let me know and I'll add them to the list.
Linux is the first OS that supports this feature, so I am not surprised that HW support for this has been poor. Hopefully this will change going forward. BTW, note the irony that CEC is now supported for DP-to-HDMI adapters, but not for the native HDMI ports on NVIDIA/AMD/Intel GPUs.
2) CEC-Tunneling does not work (yet?) if there is an MST hub in between. I'm still researching this but this might be a limitation of MST.
Regards,
Hans
Hans Verkuil (5): drm_dp_cec: check that aux has a transfer function drm_dp_cec: add note about good MegaChips 2900 CEC support drm_dp_mst_topology: fix broken drm_dp_sideband_parse_remote_dpcd_read() drm/nouveau: add DisplayPort CEC-Tunneling-over-AUX support drm/amdgpu: add DisplayPort CEC-Tunneling-over-AUX support
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 +++++++++++-- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++ drivers/gpu/drm/drm_dp_cec.c | 18 +++++++++++++++++- drivers/gpu/drm/drm_dp_mst_topology.c | 1 + drivers/gpu/drm/nouveau/nouveau_connector.c | 17 +++++++++++++++-- 5 files changed, 46 insertions(+), 5 deletions(-)
From: Hans Verkuil hans.verkuil@cisco.com
If aux->transfer == NULL, then just return without doing anything. In that case the function is likely called for a non-(e)DP connector.
This never happened for the i915 driver, but the nouveau and amdgpu drivers need this check.
The alternative would be to add this check in those drivers before every drm_dp_cec call, but it makes sense to check it in the drm_dp_cec functions to prevent a kernel oops.
Signed-off-by: Hans Verkuil hans.verkuil@cisco.com --- drivers/gpu/drm/drm_dp_cec.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c index 988513346e9c..1407b13a8d5d 100644 --- a/drivers/gpu/drm/drm_dp_cec.c +++ b/drivers/gpu/drm/drm_dp_cec.c @@ -238,6 +238,10 @@ void drm_dp_cec_irq(struct drm_dp_aux *aux) u8 cec_irq; int ret;
+ /* No transfer function was set, so not a DP connector */ + if (!aux->transfer) + return; + mutex_lock(&aux->cec.lock); if (!aux->cec.adap) goto unlock; @@ -293,6 +297,10 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid) unsigned int num_las = 1; u8 cap;
+ /* No transfer function was set, so not a DP connector */ + if (!aux->transfer) + return; + #ifndef CONFIG_MEDIA_CEC_RC /* * CEC_CAP_RC is part of CEC_CAP_DEFAULTS, but it is stripped by @@ -361,6 +369,10 @@ EXPORT_SYMBOL(drm_dp_cec_set_edid); */ void drm_dp_cec_unset_edid(struct drm_dp_aux *aux) { + /* No transfer function was set, so not a DP connector */ + if (!aux->transfer) + return; + cancel_delayed_work_sync(&aux->cec.unregister_work);
mutex_lock(&aux->cec.lock); @@ -404,6 +416,8 @@ void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name, struct device *parent) { WARN_ON(aux->cec.adap); + if (WARN_ON(!aux->transfer)) + return; aux->cec.name = name; aux->cec.parent = parent; INIT_DELAYED_WORK(&aux->cec.unregister_work,
From: Hans Verkuil hans.verkuil@cisco.com
A big problem with DP CEC-Tunneling-over-AUX is that it is tricky to find adapters with a chipset that supports this AND where the manufacturer actually connected the HDMI CEC line to the chipset.
Add a mention of the MegaChips 2900 chipset which seems to support this feature well.
Signed-off-by: Hans Verkuil hans.verkuil@cisco.com --- drivers/gpu/drm/drm_dp_cec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c index 1407b13a8d5d..8a718f85079a 100644 --- a/drivers/gpu/drm/drm_dp_cec.c +++ b/drivers/gpu/drm/drm_dp_cec.c @@ -16,7 +16,9 @@ * here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters * have a converter chip that supports CEC-Tunneling-over-AUX (usually the * Parade PS176), but they do not wire up the CEC pin, thus making CEC - * useless. + * useless. Note that MegaChips 2900-based adapters appear to have good + * support for CEC tunneling. Those adapters that I have tested using + * this chipset all have the CEC line connected. * * Sadly there is no way for this driver to know this. What happens is * that a /dev/cecX device is created that is isolated and unable to see
From: Hans Verkuil hans.verkuil@cisco.com
When parsing the reply of a DP_REMOTE_DPCD_READ DPCD command the result is wrong due to a missing idx increment.
This was never noticed since DP_REMOTE_DPCD_READ is currently not used, but if you enable it, then it is all wrong.
Signed-off-by: Hans Verkuil hans.verkuil@cisco.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 7780567aa669..5ff1d79b86c4 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -439,6 +439,7 @@ static bool drm_dp_sideband_parse_remote_dpcd_read(struct drm_dp_sideband_msg_rx if (idx > raw->curlen) goto fail_len; repmsg->u.remote_dpcd_read_ack.num_bytes = raw->msg[idx]; + idx++; if (idx > raw->curlen) goto fail_len;
From: Hans Verkuil hans.verkuil@cisco.com
Add DisplayPort CEC-Tunneling-over-AUX support to nouveau.
Signed-off-by: Hans Verkuil hans.verkuil@cisco.com --- drivers/gpu/drm/nouveau/nouveau_connector.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index 51932c72334e..eb4f766b5958 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -400,8 +400,10 @@ nouveau_connector_destroy(struct drm_connector *connector) kfree(nv_connector->edid); drm_connector_unregister(connector); drm_connector_cleanup(connector); - if (nv_connector->aux.transfer) + if (nv_connector->aux.transfer) { + drm_dp_cec_unregister_connector(&nv_connector->aux); drm_dp_aux_unregister(&nv_connector->aux); + } kfree(connector); }
@@ -608,6 +610,7 @@ nouveau_connector_detect(struct drm_connector *connector, bool force)
nouveau_connector_set_encoder(connector, nv_encoder); conn_status = connector_status_connected; + drm_dp_cec_set_edid(&nv_connector->aux, nv_connector->edid); goto out; }
@@ -1108,11 +1111,14 @@ nouveau_connector_hotplug(struct nvif_notify *notify)
if (rep->mask & NVIF_NOTIFY_CONN_V0_IRQ) { NV_DEBUG(drm, "service %s\n", name); + drm_dp_cec_irq(&nv_connector->aux); if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP))) nv50_mstm_service(nv_encoder->dp.mstm); } else { bool plugged = (rep->mask != NVIF_NOTIFY_CONN_V0_UNPLUG);
+ if (!plugged) + drm_dp_cec_unset_edid(&nv_connector->aux); NV_DEBUG(drm, "%splugged %s\n", plugged ? "" : "un", name); if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP))) { if (!plugged) @@ -1302,7 +1308,6 @@ nouveau_connector_create(struct drm_device *dev, int index) kfree(nv_connector); return ERR_PTR(ret); } - funcs = &nouveau_connector_funcs; break; default: @@ -1356,6 +1361,14 @@ nouveau_connector_create(struct drm_device *dev, int index) break; }
+ switch (type) { + case DRM_MODE_CONNECTOR_DisplayPort: + case DRM_MODE_CONNECTOR_eDP: + drm_dp_cec_register_connector(&nv_connector->aux, + connector->name, dev->dev); + break; + } + ret = nvif_notify_init(&disp->disp.object, nouveau_connector_hotplug, true, NV04_DISP_NTFY_CONN, &(struct nvif_notify_conn_req_v0) {
From: Hans Verkuil hans.verkuil@cisco.com
Add DisplayPort CEC-Tunneling-over-AUX support to amdgpu.
Signed-off-by: Hans Verkuil hans.verkuil@cisco.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 +++++++++++-- .../drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++ 2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 34f34823bab5..77898c95bef6 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -898,6 +898,7 @@ amdgpu_dm_update_connector_after_detect(struct amdgpu_dm_connector *aconnector) aconnector->dc_sink = sink; if (sink->dc_edid.length == 0) { aconnector->edid = NULL; + drm_dp_cec_unset_edid(&aconnector->dm_dp_aux.aux); } else { aconnector->edid = (struct edid *) sink->dc_edid.raw_edid; @@ -905,10 +906,13 @@ amdgpu_dm_update_connector_after_detect(struct amdgpu_dm_connector *aconnector)
drm_connector_update_edid_property(connector, aconnector->edid); + drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux, + aconnector->edid); } amdgpu_dm_add_sink_to_freesync_module(connector, aconnector->edid);
} else { + drm_dp_cec_unset_edid(&aconnector->dm_dp_aux.aux); amdgpu_dm_remove_sink_from_freesync_module(connector); drm_connector_update_edid_property(connector, NULL); aconnector->num_modes = 0; @@ -1059,12 +1063,16 @@ static void handle_hpd_rx_irq(void *param) drm_kms_helper_hotplug_event(dev); } } + if ((dc_link->cur_link_settings.lane_count != LANE_COUNT_UNKNOWN) || - (dc_link->type == dc_connection_mst_branch)) + (dc_link->type == dc_connection_mst_branch)) { dm_handle_hpd_rx_irq(aconnector); + }
- if (dc_link->type != dc_connection_mst_branch) + if (dc_link->type != dc_connection_mst_branch) { + drm_dp_cec_irq(&aconnector->dm_dp_aux.aux); mutex_unlock(&aconnector->hpd_lock); + } }
static void register_hpd_handlers(struct amdgpu_device *adev) @@ -2732,6 +2740,7 @@ static void amdgpu_dm_connector_destroy(struct drm_connector *connector) dm->backlight_dev = NULL; } #endif + drm_dp_cec_unregister_connector(&aconnector->dm_dp_aux.aux); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 9a300732ba37..18a3a6e5ffa0 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -496,6 +496,8 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc;
drm_dp_aux_register(&aconnector->dm_dp_aux.aux); + drm_dp_cec_register_connector(&aconnector->dm_dp_aux.aux, + aconnector->base.name, dm->adev->dev); aconnector->mst_mgr.cbs = &dm_mst_cbs; drm_dp_mst_topology_mgr_init( &aconnector->mst_mgr,
On Thu, Aug 16, 2018 at 6:56 AM Hans Verkuil hverkuil@xs4all.nl wrote:
From: Hans Verkuil hans.verkuil@cisco.com
Now that the DisplayPort CEC-Tunneling-over-AUX drm+i915 support has been merged in the mainline kernel it is time to roll this out to nouveau and amdgpu as well.
I combined both in the same patch series since both depend on the same first patch, the comments in this cover letter apply to both and the implementation is also very similar (and simple).
As mentioned, the first patch is required for this: it adds checks that the drm_dp_cec functions are called with a working aux implementation. These checks weren't necessary for the i915, but nouveau and amdgpu require them.
The next two patches update a comment in drm_dp_cec.c and fix a bug in sideband AUX handling that I found while researching CEC Tunneling over an MST hub. It's there to prevent others from stumbling over the same bug in the future.
The fourth patch adds support for CEC to the nouveau driver.
The last patch adds support for CEC to the amdgpu driver. However, there appear to be two classes of amdgpu hardware: as a discrete GPU or integrated. I only have a discrete GPU, so I can't test the integrated GPU support and I only implemented this for the discrete GPU case.
If someone has the integrated GPU and wants to get this working and is willing to do some testing, then please contact me. It shouldn't be difficult. You will likely have to buy a working DP-to-HDMI adapter. See https://hverkuil.home.xs4all.nl/cec-status.txt for a (sadly very short) list of working adapters.
Actually you added support for APUs as well. In amdgpu, there are two sets of modesetting code, an older less featured version (amd/amdgpu/dce*.c) and the newer more featured code (amd/display/*). Newer asics (vega and raven) are only supported by DC. Older asics are supported by both. Eventually we'd like to remove the older modesetting code. I'm not really a CEC expert, but the patches look pretty straight forward to me. Series is: Acked-by: Alex Deucher alexander.deucher@amd.com
Note that I may be completely off-base regarding what atombios_dp.c does, it's the first time I ever looked at amdgpu code.
Two notes on CEC-Tunneling-over-AUX:
You need to be very careful about which USB-C/DP/mini-DP to HDMI adapters you buy. Only a few support this feature correctly today. Known chipsets that support this are Parade PS175 & PS176 and MegaChips 2900. Unfortunately almost all Parade-based adapters do not hook up the HDMI CEC pin to the chip, making them useless for this. The Parade reference design was only recently changed to hook up this pin, so perhaps this situation will change for new Parade-based adapters.
Adapters based on the new MegaChips 2900 fare much better: it appears that their reference design *does* hook up the CEC pin. Club3D has adapters using this device for USB-C, DP and mini-DP to HDMI, and they all work fine.
If anyone finds other adapters that work besides those I list in https://hverkuil.home.xs4all.nl/cec-status.txt, please let me know and I'll add them to the list.
Linux is the first OS that supports this feature, so I am not surprised that HW support for this has been poor. Hopefully this will change going forward. BTW, note the irony that CEC is now supported for DP-to-HDMI adapters, but not for the native HDMI ports on NVIDIA/AMD/Intel GPUs.
CEC-Tunneling does not work (yet?) if there is an MST hub in between. I'm still researching this but this might be a limitation of MST.
Regards,
HansHans Verkuil (5): drm_dp_cec: check that aux has a transfer function drm_dp_cec: add note about good MegaChips 2900 CEC support drm_dp_mst_topology: fix broken drm_dp_sideband_parse_remote_dpcd_read() drm/nouveau: add DisplayPort CEC-Tunneling-over-AUX support drm/amdgpu: add DisplayPort CEC-Tunneling-over-AUX support
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 +++++++++++-- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++ drivers/gpu/drm/drm_dp_cec.c | 18 +++++++++++++++++- drivers/gpu/drm/drm_dp_mst_topology.c | 1 + drivers/gpu/drm/nouveau/nouveau_connector.c | 17 +++++++++++++++-- 5 files changed, 46 insertions(+), 5 deletions(-)
-- 2.18.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Hans Verkuil