media: cec: DRM connector to CEC dev mapping
Subsystems (8)
RFC PATCH 1/1 media: cec: DRM connector to CEC dev mapping
This patch proposes to introduce explicit mapping between DRM connectors and /dev/cecX adapters.
This is achieved here by adding a new structure with connector info (DRM card number and connector id) to cec_adapter. That connector info is expected to be set either by a code creating an adapter or trough a CEC notifier (which is extended for that purpose).
A new ioctl, exposing connector info to userland, is added to /dev/cec. New CEC event type is also added. That event is to notify an application that the info about mapping between an adapter and a DRM connector was just set.
---
Notes:
Sending the patch in its current form the get feedback if that is considered a right approach or if some other way of learning the mapping in userland is preferred.
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- drivers/gpu/drm/drm_dp_cec.c | 20 ++++---- drivers/gpu/drm/i915/intel_dp.c | 4 +- drivers/gpu/drm/i915/intel_hdmi.c | 7 ++- drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +- drivers/media/cec/cec-adap.c | 21 ++++++++- drivers/media/cec/cec-api.c | 17 +++++++ drivers/media/cec/cec-core.c | 12 ++++- drivers/media/cec/cec-notifier.c | 47 ++++++++++++++++--- include/drm/drm_dp_helper.h | 14 +++--- include/media/cec-notifier.h | 31 ++++++++++-- include/media/cec.h | 13 ++++- include/uapi/linux/cec.h | 18 +++++++ 13 files changed, 172 insertions(+), 37 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +1 -1
@@ -379,7 +379,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,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->base); aconnector->mst_mgr.cbs = &dm_mst_cbs; drm_dp_mst_topology_mgr_init( &aconnector->mst_mgr,
drivers/gpu/drm/drm_dp_cec.c +12 -8
@@ -8,7 +8,9 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/slab.h> +#include <drm/drm_connector.h> #include <drm/drm_dp_helper.h> +#include <drm/drmP.h> #include <media/cec.h> /*
@@ -295,6 +297,7 @@ static void drm_dp_cec_unregister_work(struct work_struct *work)*/ void drm_dp_cec_set_edid (struct drm_dp_aux *aux,const struct edid *edid) { +struct drm_connector *connector = aux->cec.connector;u32 cec_caps =CEC_CAP_DEFAULTS |CEC_CAP_NEEDS_HPD ;unsigned int num_las =1 ;u8 cap;
@@ -344,16 +347,19 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)/* Create a new adapter */ aux->cec.adap =cec_allocate_adapter (&drm_dp_cec_adap_ops, - aux, aux->cec.name, cec_caps, + aux, connector->name, cec_caps, num_las);if (IS_ERR (aux->cec.adap)) { aux->cec.adap =NULL ; goto unlock; } -if (cec_register_adapter (aux->cec.adap, aux->cec.parent)) { +if (cec_register_adapter (aux->cec.adap, connector->dev->dev)) {cec_delete_adapter (aux->cec.adap); aux->cec.adap =NULL ; }else { +cec_s_connector_info (aux->cec.adap, + connector->dev->primary->index, + connector->base.id);/* * Update the phys addr for the new CEC adapter. When called * from drm_dp_cec_register_connector() edid == NULL, so in
@@ -406,22 +412,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);/** * drm_dp_cec_register_connector() - register a new connector * @aux: DisplayPort AUX channel -* @name: name of the CEC device -* @parent: parent device +* @connector: drm connector * * A new connector was registered with associated CEC adapter name and * CEC adapter parent device. After registering the name and parent * drm_dp_cec_set_edid() is called to check if the connector supports * CEC and to register a CEC adapter if that is the case. */ -void drm_dp_cec_register_connector (struct drm_dp_aux *aux,const char *name, -struct device *parent) +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector) {WARN_ON (aux->cec.adap);if (WARN_ON (!aux->transfer))return ; - aux->cec.name = name; - aux->cec.parent = parent; + aux->cec.connector = connector;INIT_DELAYED_WORK (&aux->cec.unregister_work, drm_dp_cec_unregister_work); }
drivers/gpu/drm/i915/intel_dp.c +2 -2
@@ -5557,8 +5557,7 @@ intel_dp_connector_register(struct drm_connector *connector) intel_dp->aux.dev = connector->kdev; ret =drm_dp_aux_register (&intel_dp->aux);if (!ret) -drm_dp_cec_register_connector (&intel_dp->aux, - connector->name, dev->dev); +drm_dp_cec_register_connector (&intel_dp->aux, connector);return ret; }
@@ -7107,3 +7106,4 @@ void intel_dp_mst_resume(struct drm_i915_private *dev_priv) } } } +
drivers/gpu/drm/i915/intel_hdmi.c +6 -1
@@ -2448,8 +2448,13 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, intel_hdmi->cec_notifier =cec_notifier_get_conn (dev->dev,port_identifier (port)); -if (!intel_hdmi->cec_notifier) +if (intel_hdmi->cec_notifier) { +cec_notifier_set_connector_info (intel_hdmi->cec_notifier, + connector->dev->primary->index, + connector->base.id); + }else {DRM_DEBUG_KMS ("CEC notifier get failed\n" ); + } } void intel_hdmi_init(struct drm_i915_private *dev_priv,
drivers/gpu/drm/nouveau/nouveau_connector.c +1 -2
@@ -1413,8 +1413,7 @@ nouveau_connector_create(struct drm_device *dev,switch (type) {case DRM_MODE_CONNECTOR_DisplayPort:case DRM_MODE_CONNECTOR_eDP: -drm_dp_cec_register_connector (&nv_connector->aux, - connector->name, dev->dev); +drm_dp_cec_register_connector (&nv_connector->aux, connector);break ; }
drivers/media/cec/cec-adap.c +20 -1
@@ -87,7 +87,7 @@ void cec_queue_event_fh(struct cec_fh *fh, const struct cec_event *new_ev, u64 ts) {static const u16 max_events[CEC_NUM_EVENTS ] = { -1 ,1 ,800 ,800 ,8 ,8 ,8 ,8 +1 ,1 ,800 ,800 ,8 ,8 ,8 ,8 ,1 };struct cec_event_entry *entry;unsigned int ev_idx = new_ev->event -1 ;
@@ -1566,6 +1566,25 @@ void cec_s_phys_addr_from_edid(struct cec_adapter *adap, }EXPORT_SYMBOL_GPL (cec_s_phys_addr_from_edid); +void cec_s_connector_info (struct cec_adapter *adap,int card_no, +u32 connector_id) +{ +struct cec_event ev = { + .event =CEC_EVENT_CONNECTOR_SET , + .flags =0 , + .connector.card_no = card_no, + .connector.connector_id = connector_id, + }; + +mutex_lock (&adap->lock); + adap->connector.card_no = card_no; + adap->connector.connector_id = connector_id; +mutex_unlock (&adap->lock); + +cec_queue_event (adap, &ev); +} +EXPORT_SYMBOL_GPL (cec_s_connector_info); +/* * Called from either the ioctl or a driver to set the logical addresses. *
drivers/media/cec/cec-api.c +17 -0
@@ -187,6 +187,20 @@ static long cec_adap_s_log_addrs(struct cec_adapter *adap, struct cec_fh *fh,return 0 ; } +static long cec_adap_g_connector_info (struct cec_adapter *adap, +struct cec_log_addrs __user *parg) +{ +struct cec_connector_info connector_info; + +mutex_lock (&adap->lock); + connector_info = adap->connector; +mutex_unlock (&adap->lock); + +if (copy_to_user (parg, &connector_info,sizeof (connector_info))) +return -EFAULT ; +return 0 ; +} +static long cec_transmit (struct cec_adapter *adap,struct cec_fh *fh,bool block,struct cec_msg __user *parg) {
@@ -514,6 +528,9 @@ static long cec_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)case CEC_ADAP_S_LOG_ADDRS :return cec_adap_s_log_addrs (adap, fh, block, parg); +case CEC_ADAP_G_CONNECTOR_INFO : +return cec_adap_g_connector_info (adap, parg); +case CEC_TRANSMIT :return cec_transmit (adap, fh, block, parg);
drivers/media/cec/cec-core.c +10 -2
@@ -182,11 +182,17 @@ static void cec_devnode_unregister(struct cec_adapter *adap) }#ifdef CONFIG_CEC_NOTIFIER -static void cec_cec_notify (struct cec_adapter *adap,u16 pa) +static void cec_cec_phys_addr_notify (struct cec_adapter *adap,u16 pa) {cec_s_phys_addr (adap, pa, false); } +static void cec_cec_connector_notify (struct cec_adapter *adap,int card_no, +u32 connector_id) +{ +cec_s_connector_info (adap, card_no, connector_id); +} +void cec_register_cec_notifier (struct cec_adapter *adap,struct cec_notifier *notifier) {
@@ -194,7 +200,9 @@ void cec_register_cec_notifier(struct cec_adapter *adap,return ; adap->notifier = notifier; - cec_notifier_register(adap->notifier, adap, cec_cec_notify); +cec_notifier_register (adap->notifier, adap, + cec_cec_phys_addr_notify, + cec_cec_connector_notify); }EXPORT_SYMBOL_GPL (cec_register_cec_notifier);#endif
drivers/media/cec/cec-notifier.c +40 -7
85 lines not on this page · raw message
include/drm/drm_dp_helper.h +7 -7
@@ -1208,6 +1208,7 @@ struct drm_dp_aux_msg {struct cec_adapter ;struct edid ; +struct drm_connector ;/** * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
@@ -1220,8 +1221,7 @@ struct edid;struct drm_dp_aux_cec {struct mutex lock;struct cec_adapter *adap; -const char *name; -struct device *parent; +struct drm_connector *connector;struct delayed_work unregister_work; };
@@ -1418,8 +1418,8 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)#ifdef CONFIG_DRM_DP_CEC void drm_dp_cec_irq (struct drm_dp_aux *aux); -void drm_dp_cec_register_connector (struct drm_dp_aux *aux,const char *name, -struct device *parent); +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector);void drm_dp_cec_unregister_connector (struct drm_dp_aux *aux);void drm_dp_cec_set_edid (struct drm_dp_aux *aux,const struct edid *edid);void drm_dp_cec_unset_edid (struct drm_dp_aux *aux);
@@ -1428,9 +1428,9 @@ static inline void drm_dp_cec_irq(struct drm_dp_aux *aux) { } -static inline void drm_dp_cec_register_connector (struct drm_dp_aux *aux, -const char *name, -struct device *parent) +static inline +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector) { }
include/media/cec-notifier.h +27 -4
@@ -63,15 +63,28 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa);void cec_notifier_set_phys_addr_from_edid (struct cec_notifier *n,const struct edid *edid); +/** +* cec_notifier_set_connector_info - associates connector info with a notifier +* @n: the CEC notifier +* @card_no: the drm card number +* @connector_id: drm connector id +*/ +void cec_notifier_set_connector_info (struct cec_notifier *n,int card_no, +u32 connector_id);/** * cec_notifier_register - register a callback with the notifier * @n: the CEC notifier * @adap: the CEC adapter, passed as argument to the callback function -* @callback: the callback function +* @phys_addr_callback : callback invoked when new phys address is set +* @connector_callback : callback invoked when connector is set */ void cec_notifier_register (struct cec_notifier *n,struct cec_adapter *adap, -void (*callback)(struct cec_adapter *adap,u16 pa)); +void (*phys_addr_callback)(struct cec_adapter *adap, +u16 pa), +void (*connector_callback)(struct cec_adapter *adap, +int card_no, +u32 connector_id));/** * cec_notifier_unregister - unregister the callback from the notifier.
@@ -108,9 +121,19 @@ static inline void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n, { } +static inline void cec_notifier_set_connector_info (struct cec_notifier *n, +int card_no, +u32 connector_id) +{ +} +static inline void cec_notifier_register (struct cec_notifier *n, -struct cec_adapter *adap, -void (*callback)(struct cec_adapter *adap,u16 pa)) +struct cec_adapter *adap, +void (*phys_addr_callback)(struct cec_adapter *adap, +u16 pa), +void (*connector_callback)(struct cec_adapter *adap, +int card_no, +u32 connector_id)) { }
include/media/cec.h +11 -2
@@ -78,7 +78,7 @@ struct cec_event_entry { };#define CEC_NUM_CORE_EVENTS 2 -#define CEC_NUM_EVENTS CEC_EVENT_PIN_5V_HIGH +#define CEC_NUM_EVENTS CEC_EVENT_CONNECTOR_SETstruct cec_fh {struct list_head list;
@@ -200,6 +200,8 @@ struct cec_adapter {u32 sequence;char input_phys[32 ]; + +struct cec_connector_info connector; };static inline void *cec_get_drvdata (const struct cec_adapter *adap)
@@ -247,9 +249,11 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block);void cec_s_phys_addr_from_edid (struct cec_adapter *adap,const struct edid *edid); +void cec_s_connector_info (struct cec_adapter *adap,int card_no, +u32 connector_id); +int cec_transmit_msg (struct cec_adapter *adap,struct cec_msg *msg,bool block); -/* Called by the adapter */ void cec_transmit_done_ts(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt, u8 nack_cnt, u8 low_drive_cnt,
@@ -357,6 +361,11 @@ static inline void cec_s_phys_addr_from_edid(struct cec_adapter *adap, { } +void cec_s_connector_info (struct cec_adapter *adap,int card_no, +u32 connector_id) +{ +} +static inline u16 cec_get_edid_phys_addr (const u8 *edid,unsigned int size,unsigned int *offset) {
include/uapi/linux/cec.h +18 -0
@@ -389,6 +389,7 @@ struct cec_log_addrs {#define CEC_EVENT_PIN_HPD_HIGH 6#define CEC_EVENT_PIN_5V_LOW 7#define CEC_EVENT_PIN_5V_HIGH 8 +#define CEC_EVENT_CONNECTOR_SET 9#define CEC_EVENT_FL_INITIAL_STATE (1 << 0)#define CEC_EVENT_FL_DROPPED_EVENTS (1 << 1)
@@ -411,6 +412,19 @@ struct cec_event_lost_msgs {__u32 lost_msgs; }; +/** +* struct cec_event_connector - tells if and which drm connector is associated +* with the CEC adapter. +* @card_no: drm card number, -1 if no connector +* @connector_id: drm connector id. +*/ + +#define CEC_CONNECTOR_INVALID_CARD_NO -1 +struct cec_connector_info { +int card_no; +__u32 connector_id; +}; +/** * struct cec_event - CEC event structure * @ts: the timestamp of when the event was sent.
@@ -427,6 +441,7 @@ struct cec_event {union {struct cec_event_state_change state_change;struct cec_event_lost_msgs lost_msgs; +struct cec_connector_info connector;__u32 raw[16 ]; }; };
@@ -475,6 +490,9 @@ struct cec_event {#define CEC_G_MODE _IOR('a', 8, __u32)#define CEC_S_MODE _IOW('a', 9, __u32) +/* Gets the connector info */ +#define CEC_ADAP_G_CONNECTOR_INFO _IOR('a', 10, struct cec_connector_info) +/* * The remainder of this header defines all CEC messages and operands. * The format matters since it the cec-ctl utility parses it to generate
Signature · 1 line
2.18.1
Hi Dariusz,
Quoted 13 lines · Dariusz Marcinkiewicz
This patch proposes to introduce explicit mapping between DRM connectors and
/dev/cecXadapters.This is achieved here by adding a new structure with connector info (DRM card number and connector id) to
cec_adapter. That connector info is expected to be set either by a code creating an adapter or trough a CEC notifier (which is extended for that purpose).A new ioctl, exposing connector info to userland, is added to
/dev/cec. New CEC event type is also added. That event is to notify an application that the info about mapping between an adapter and a DRM connector was just set.
Quoted 7 lines
Signed-off-by Dariusz Marcinkiewicz <[email protected]>---
Notes:
Sending the patch in its current form the get feedback if that is considered a right approach or if some other way of learning the mapping in userland is preferred.
Thank you for this patch. It's interesting, but I think it should be redesigned a bit.
Basically what you want to do is to provide a way for userspace to associate the CEC device with the HDMI connector it is associated with.
But this information is known the moment the cec adapter is allocated or the cec_notifier is registered for a connector. So I would provide this information at that time, rather than through separate functions. This makes everything nicely atomic. This also avoids the need of a connector_info event, since the connector info is known from the very beginning. You still need the ioctl to obtain this information, though.
There are also two related issues.
Firstly, CEC is used not only for DRM, but also for V4L2, and it makes sense to provide connector info there as well. For V4L2 this would probably look like this (tentative as I am not completely sure about the major/minor numbers):
struct v4l2_connector_info {
__u32 major;
__u32 minor;
__u16 connector_mask;
}Where the major/minor numbers determine the video or v4l-subdev device that controls the HDMI connector(s). Since a CEC device for HDMI receivers can be responsible for multiple HDMI inputs we need the connector_mask to indicate which HDMI inputs it is associated with.
Secondly, there are USB-CEC dongles that are not directly associated with an HDMI connector since the driver simply doesn't know. But it would be nice if the user can provide that information. I did a proof-of-concept here:
https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=pulse8-notifier
and it works fine. But the problem is that the connector name is currently only available on the i915 via port_identifier(port), and that that port identifier string is not visible to the end-user. I had to try various port names ("Port A", "Port B", etc) before I hit the right one.
But as I understand it that port identifier name is stable, and that's what is needed in a driver like seco-cec.c (used for UDOO x86 boards) to correctly create the CEC device for the right HDMI connector.
It would be nice to have something that works for all these cases.
I'm especially not keen on the port_identifier(port) thing since it is i915 specific and something similar on the level of the DRM API would be much more useful.
I'm not a drm specialist, though.
Regards,
Hans
Quoted 555 lines
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- drivers/gpu/drm/drm_dp_cec.c | 20 ++++---- drivers/gpu/drm/i915/intel_dp.c | 4 +- drivers/gpu/drm/i915/intel_hdmi.c | 7 ++- drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +- drivers/media/cec/cec-adap.c | 21 ++++++++- drivers/media/cec/cec-api.c | 17 +++++++ drivers/media/cec/cec-core.c | 12 ++++- drivers/media/cec/cec-notifier.c | 47 ++++++++++++++++--- include/drm/drm_dp_helper.h | 14 +++--- include/media/cec-notifier.h | 31 ++++++++++-- include/media/cec.h | 13 ++++- include/uapi/linux/cec.h | 18 +++++++ 13 files changed, 172 insertions(+), 37 deletions(-)drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +1
-1@@ -379,7 +379,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,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->base); aconnector->mst_mgr.cbs = &dm_mst_cbs; drm_dp_mst_topology_mgr_init( &aconnector->mst_mgr,drivers/gpu/drm/drm_dp_cec.c +12
-8@@ -8,7 +8,9 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/slab.h> +#include <drm/drm_connector.h> #include <drm/drm_dp_helper.h> +#include <drm/drmP.h> #include <media/cec.h> /* @@ -295,6 +297,7 @@ static void drm_dp_cec_unregister_work(struct work_struct *work)*/ void drm_dp_cec_set_edid (struct drm_dp_aux *aux,const struct edid *edid) { +struct drm_connector *connector = aux->cec.connector;u32 cec_caps =CEC_CAP_DEFAULTS |CEC_CAP_NEEDS_HPD ;unsigned int num_las =1 ;u8 cap;@@ -344,16 +347,19 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)/* Create a new adapter */ aux->cec.adap =cec_allocate_adapter (&drm_dp_cec_adap_ops, - aux, aux->cec.name, cec_caps, + aux, connector->name, cec_caps, num_las);if (IS_ERR (aux->cec.adap)) { aux->cec.adap =NULL ; goto unlock; } -if (cec_register_adapter (aux->cec.adap, aux->cec.parent)) { +if (cec_register_adapter (aux->cec.adap, connector->dev->dev)) {cec_delete_adapter (aux->cec.adap); aux->cec.adap =NULL ; }else { +cec_s_connector_info (aux->cec.adap, + connector->dev->primary->index, + connector->base.id);/* * Update the phys addr for the new CEC adapter. When called * from drm_dp_cec_register_connector() edid == NULL, so in @@ -406,22 +412,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);/** * drm_dp_cec_register_connector() - register a new connector * @aux: DisplayPort AUX channel -* @name: name of the CEC device -* @parent: parent device +* @connector: drm connector * * A new connector was registered with associated CEC adapter name and * CEC adapter parent device. After registering the name and parent * drm_dp_cec_set_edid() is called to check if the connector supports * CEC and to register a CEC adapter if that is the case. */ -void drm_dp_cec_register_connector (struct drm_dp_aux *aux,const char *name, -struct device *parent) +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector) {WARN_ON (aux->cec.adap);if (WARN_ON (!aux->transfer))return ; - aux->cec.name = name; - aux->cec.parent = parent; + aux->cec.connector = connector;INIT_DELAYED_WORK (&aux->cec.unregister_work, drm_dp_cec_unregister_work); }drivers/gpu/drm/i915/intel_dp.c +2
-2@@ -5557,8 +5557,7 @@ intel_dp_connector_register(struct drm_connector *connector) intel_dp->aux.dev = connector->kdev; ret =drm_dp_aux_register (&intel_dp->aux);if (!ret) -drm_dp_cec_register_connector (&intel_dp->aux, - connector->name, dev->dev); +drm_dp_cec_register_connector (&intel_dp->aux, connector);return ret; }@@ -7107,3 +7106,4 @@ void intel_dp_mst_resume(struct drm_i915_private *dev_priv) } } } +drivers/gpu/drm/i915/intel_hdmi.c +6
-1@@ -2448,8 +2448,13 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, intel_hdmi->cec_notifier =cec_notifier_get_conn (dev->dev,port_identifier (port)); -if (!intel_hdmi->cec_notifier) +if (intel_hdmi->cec_notifier) { +cec_notifier_set_connector_info (intel_hdmi->cec_notifier, + connector->dev->primary->index, + connector->base.id); + }else {DRM_DEBUG_KMS ("CEC notifier get failed\n" ); + } } void intel_hdmi_init(struct drm_i915_private *dev_priv,drivers/gpu/drm/nouveau/nouveau_connector.c +1
-2@@ -1413,8 +1413,7 @@ nouveau_connector_create(struct drm_device *dev,switch (type) {case DRM_MODE_CONNECTOR_DisplayPort:case DRM_MODE_CONNECTOR_eDP: -drm_dp_cec_register_connector (&nv_connector->aux, - connector->name, dev->dev); +drm_dp_cec_register_connector (&nv_connector->aux, connector);break ; }drivers/media/cec/cec-adap.c +20
-1@@ -87,7 +87,7 @@ void cec_queue_event_fh(struct cec_fh *fh, const struct cec_event *new_ev, u64 ts) {static const u16 max_events[CEC_NUM_EVENTS ] = { -1 ,1 ,800 ,800 ,8 ,8 ,8 ,8 +1 ,1 ,800 ,800 ,8 ,8 ,8 ,8 ,1 };struct cec_event_entry *entry;unsigned int ev_idx = new_ev->event -1 ;@@ -1566,6 +1566,25 @@ void cec_s_phys_addr_from_edid(struct cec_adapter *adap, }EXPORT_SYMBOL_GPL (cec_s_phys_addr_from_edid); +void cec_s_connector_info (struct cec_adapter *adap,int card_no, +u32 connector_id) +{ +struct cec_event ev = { + .event =CEC_EVENT_CONNECTOR_SET , + .flags =0 , + .connector.card_no = card_no, + .connector.connector_id = connector_id, + }; + +mutex_lock (&adap->lock); + adap->connector.card_no = card_no; + adap->connector.connector_id = connector_id; +mutex_unlock (&adap->lock); + +cec_queue_event (adap, &ev); +} +EXPORT_SYMBOL_GPL (cec_s_connector_info); +/* * Called from either the ioctl or a driver to set the logical addresses. * drivers/media/cec/cec-api.c +17
-0@@ -187,6 +187,20 @@ static long cec_adap_s_log_addrs(struct cec_adapter *adap, struct cec_fh *fh,return 0 ; } +static long cec_adap_g_connector_info (struct cec_adapter *adap, +struct cec_log_addrs __user *parg) +{ +struct cec_connector_info connector_info; + +mutex_lock (&adap->lock); + connector_info = adap->connector; +mutex_unlock (&adap->lock); + +if (copy_to_user (parg, &connector_info,sizeof (connector_info))) +return -EFAULT ; +return 0 ; +} +static long cec_transmit (struct cec_adapter *adap,struct cec_fh *fh,bool block,struct cec_msg __user *parg) {@@ -514,6 +528,9 @@ static long cec_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)case CEC_ADAP_S_LOG_ADDRS :return cec_adap_s_log_addrs (adap, fh, block, parg); +case CEC_ADAP_G_CONNECTOR_INFO : +return cec_adap_g_connector_info (adap, parg); +case CEC_TRANSMIT :return cec_transmit (adap, fh, block, parg);drivers/media/cec/cec-core.c +10
-2@@ -182,11 +182,17 @@ static void cec_devnode_unregister(struct cec_adapter *adap) }#ifdef CONFIG_CEC_NOTIFIER -static void cec_cec_notify (struct cec_adapter *adap,u16 pa) +static void cec_cec_phys_addr_notify (struct cec_adapter *adap,u16 pa) {cec_s_phys_addr (adap, pa, false); } +static void cec_cec_connector_notify (struct cec_adapter *adap,int card_no, +u32 connector_id) +{ +cec_s_connector_info (adap, card_no, connector_id); +} +void cec_register_cec_notifier (struct cec_adapter *adap,struct cec_notifier *notifier) {@@ -194,7 +200,9 @@ void cec_register_cec_notifier(struct cec_adapter *adap,return ; adap->notifier = notifier; - cec_notifier_register(adap->notifier, adap, cec_cec_notify); +cec_notifier_register (adap->notifier, adap, + cec_cec_phys_addr_notify, + cec_cec_connector_notify); }EXPORT_SYMBOL_GPL (cec_register_cec_notifier);#endif drivers/media/cec/cec-notifier.c +40
-7@@ -23,9 +23,13 @@ struct cec_notifier {struct device *dev;const char *conn;struct cec_adapter *cec_adap; -void (*callback)(struct cec_adapter *adap,u16 pa); +void (*phys_addr_callback)(struct cec_adapter *adap,u16 pa); +void (*connector_callback)(struct cec_adapter *adap, +int card_no, +u32 connector_id);u16 phys_addr; +struct cec_connector_info connector_info; };static LIST_HEAD (cec_notifiers);@@ -51,6 +55,7 @@ struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char *conn)if (conn) n->conn =kstrdup (conn,GFP_KERNEL ); n->phys_addr =CEC_PHYS_ADDR_INVALID ; + n->connector_info.card_no =CEC_CONNECTOR_INVALID_CARD_NO ;mutex_init (&n->lock);kref_init (&n->kref);list_add_tail (&n->head, &cec_notifiers);@@ -85,8 +90,8 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa)mutex_lock (&n->lock); n->phys_addr = pa; -if (n->callback) - n->callback (n->cec_adap, n->phys_addr); +if (n->phys_addr_callback) + n->phys_addr_callback (n->cec_adap, n->phys_addr);mutex_unlock (&n->lock); }EXPORT_SYMBOL_GPL (cec_notifier_set_phys_addr);@@ -106,15 +111,42 @@ void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n, }EXPORT_SYMBOL_GPL (cec_notifier_set_phys_addr_from_edid); +void cec_notifier_set_connector_info (struct cec_notifier *n,int card_no, +u32 connector_id) +{ +if (n ==NULL ) +return ; + +mutex_lock (&n->lock); + n->connector_info.card_no = card_no; + n->connector_info.connector_id = connector_id; +if (n->connector_callback) + n->connector_callback (n->cec_adap, card_no, connector_id); +mutex_unlock (&n->lock); +} +EXPORT_SYMBOL_GPL (cec_notifier_set_connector_info); +void cec_notifier_register (struct cec_notifier *n,struct cec_adapter *adap, -void (*callback)(struct cec_adapter *adap,u16 pa)) +void (*phys_addr_callback)(struct cec_adapter *adap, +u16 pa), +void (*connector_callback)(struct cec_adapter *adap, +int card_no, +u32 connector_id)) {kref_get (&n->kref);mutex_lock (&n->lock); n->cec_adap = adap; - n->callback = callback; - n->callback (adap, n->phys_addr); + + n->phys_addr_callback = phys_addr_callback; + n->phys_addr_callback (adap, n->phys_addr); + + n->connector_callback = connector_callback; +if (n->connector_callback) { + n->connector_callback (adap, n->connector_info.card_no, + n->connector_info.connector_id); + } +mutex_unlock (&n->lock); }EXPORT_SYMBOL_GPL (cec_notifier_register);@@ -122,7 +154,8 @@ EXPORT_SYMBOL_GPL(cec_notifier_register);void cec_notifier_unregister (struct cec_notifier *n) {mutex_lock (&n->lock); - n->callback =NULL ; + n->phys_addr_callback =NULL ; + n->connector_callback =NULL ;mutex_unlock (&n->lock);cec_notifier_put (n); }include/drm/drm_dp_helper.h +7
-7@@ -1208,6 +1208,7 @@ struct drm_dp_aux_msg {struct cec_adapter ;struct edid ; +struct drm_connector ;/** * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX @@ -1220,8 +1221,7 @@ struct edid;struct drm_dp_aux_cec {struct mutex lock;struct cec_adapter *adap; -const char *name; -struct device *parent; +struct drm_connector *connector;struct delayed_work unregister_work; };@@ -1418,8 +1418,8 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)#ifdef CONFIG_DRM_DP_CEC void drm_dp_cec_irq (struct drm_dp_aux *aux); -void drm_dp_cec_register_connector (struct drm_dp_aux *aux,const char *name, -struct device *parent); +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector);void drm_dp_cec_unregister_connector (struct drm_dp_aux *aux);void drm_dp_cec_set_edid (struct drm_dp_aux *aux,const struct edid *edid);void drm_dp_cec_unset_edid (struct drm_dp_aux *aux);@@ -1428,9 +1428,9 @@ static inline void drm_dp_cec_irq(struct drm_dp_aux *aux) { } -static inline void drm_dp_cec_register_connector (struct drm_dp_aux *aux, -const char *name, -struct device *parent) +static inline +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector) { }include/media/cec-notifier.h +27
-4@@ -63,15 +63,28 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa);void cec_notifier_set_phys_addr_from_edid (struct cec_notifier *n,const struct edid *edid); +/** +* cec_notifier_set_connector_info - associates connector info with a notifier +* @n: the CEC notifier +* @card_no: the drm card number +* @connector_id: drm connector id +*/ +void cec_notifier_set_connector_info (struct cec_notifier *n,int card_no, +u32 connector_id);/** * cec_notifier_register - register a callback with the notifier * @n: the CEC notifier * @adap: the CEC adapter, passed as argument to the callback function -* @callback: the callback function +* @phys_addr_callback : callback invoked when new phys address is set +* @connector_callback : callback invoked when connector is set */ void cec_notifier_register (struct cec_notifier *n,struct cec_adapter *adap, -void (*callback)(struct cec_adapter *adap,u16 pa)); +void (*phys_addr_callback)(struct cec_adapter *adap, +u16 pa), +void (*connector_callback)(struct cec_adapter *adap, +int card_no, +u32 connector_id));/** * cec_notifier_unregister - unregister the callback from the notifier. @@ -108,9 +121,19 @@ static inline void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n, { } +static inline void cec_notifier_set_connector_info (struct cec_notifier *n, +int card_no, +u32 connector_id) +{ +} +static inline void cec_notifier_register (struct cec_notifier *n, -struct cec_adapter *adap, -void (*callback)(struct cec_adapter *adap,u16 pa)) +struct cec_adapter *adap, +void (*phys_addr_callback)(struct cec_adapter *adap, +u16 pa), +void (*connector_callback)(struct cec_adapter *adap, +int card_no, +u32 connector_id)) { }include/media/cec.h +11
-2@@ -78,7 +78,7 @@ struct cec_event_entry { };#define CEC_NUM_CORE_EVENTS 2 -#define CEC_NUM_EVENTS CEC_EVENT_PIN_5V_HIGH +#define CEC_NUM_EVENTS CEC_EVENT_CONNECTOR_SETstruct cec_fh {struct list_head list;@@ -200,6 +200,8 @@ struct cec_adapter {u32 sequence;char input_phys[32 ]; + +struct cec_connector_info connector; };static inline void *cec_get_drvdata (const struct cec_adapter *adap)@@ -247,9 +249,11 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block);void cec_s_phys_addr_from_edid (struct cec_adapter *adap,const struct edid *edid); +void cec_s_connector_info (struct cec_adapter *adap,int card_no, +u32 connector_id); +int cec_transmit_msg (struct cec_adapter *adap,struct cec_msg *msg,bool block); -/* Called by the adapter */ void cec_transmit_done_ts(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt, u8 nack_cnt, u8 low_drive_cnt,@@ -357,6 +361,11 @@ static inline void cec_s_phys_addr_from_edid(struct cec_adapter *adap, { } +void cec_s_connector_info (struct cec_adapter *adap,int card_no, +u32 connector_id) +{ +} +static inline u16 cec_get_edid_phys_addr (const u8 *edid,unsigned int size,unsigned int *offset) {include/uapi/linux/cec.h +18
-0@@ -389,6 +389,7 @@ struct cec_log_addrs {#define CEC_EVENT_PIN_HPD_HIGH 6#define CEC_EVENT_PIN_5V_LOW 7#define CEC_EVENT_PIN_5V_HIGH 8 +#define CEC_EVENT_CONNECTOR_SET 9#define CEC_EVENT_FL_INITIAL_STATE (1 << 0)#define CEC_EVENT_FL_DROPPED_EVENTS (1 << 1)@@ -411,6 +412,19 @@ struct cec_event_lost_msgs {__u32 lost_msgs; }; +/** +* struct cec_event_connector - tells if and which drm connector is associated +* with the CEC adapter. +* @card_no: drm card number, -1 if no connector +* @connector_id: drm connector id. +*/ + +#define CEC_CONNECTOR_INVALID_CARD_NO -1 +struct cec_connector_info { +int card_no; +__u32 connector_id; +}; +/** * struct cec_event - CEC event structure * @ts: the timestamp of when the event was sent. @@ -427,6 +441,7 @@ struct cec_event {union {struct cec_event_state_change state_change;struct cec_event_lost_msgs lost_msgs; +struct cec_connector_info connector;__u32 raw[16 ]; }; };@@ -475,6 +490,9 @@ struct cec_event {#define CEC_G_MODE _IOR('a', 8, __u32)#define CEC_S_MODE _IOW('a', 9, __u32) +/* Gets the connector info */ +#define CEC_ADAP_G_CONNECTOR_INFO _IOR('a', 10, struct cec_connector_info) +/* * The remainder of this header defines all CEC messages and operands. * The format matters since it the cec-ctl utility parses it to generate
Hi Hans.
Quoted 1 line · Hans Verkuil
...
Quoted 11 lines
Quoted 4 lines
Notes:
Sending the patch in its current form the get feedback if that is considered a right approach or if some other way of learning the mapping in userland is preferred.Thank you for this patch. It's interesting, but I think it should be redesigned a bit.
Basically what you want to do is to provide a way for userspace to associate the CEC device with the HDMI connector it is associated with.
Quoted 7 lines
But this information is known the moment the cec adapter is allocated or the
cec_notifieris registered for a connector. So I would provide this information at that time, rather than through separate functions. This makes everything nicely atomic. This also avoids the need of aconnector_infoevent, since the connector info is known from the very beginning. You still need the ioctl to obtain this information, though.
Great, good to know that the event can be removed. I wasn't 100% sure if in all cases it can be assumed that connector info will be set on the notifier before the CEC adapter is created, hence the event. Sending an updated patch with the event no longer there.
Quoted 12 lines
There are also two related issues.
Firstly, CEC is used not only for DRM, but also for V4L2, and it makes sense to provide connector info there as well. For V4L2 this would probably look like this (tentative as I am not completely sure about the major/minor numbers):
struct v4l2_connector_info { __u32 major; __u32 minor; __u16 connector_mask; }
Quoted 4 lines
Where the major/minor numbers determine the video or v4l-subdev device that controls the HDMI connector(s). Since a CEC device for HDMI receivers can be responsible for multiple HDMI inputs we need the
connector_maskto indicate which HDMI inputs it is associated with.
Would simply allowing for multiple connector types to be hooked up with cec adapters be sufficient to cover this? Please take a look at the follow up patch.
Quoted 5 lines
Secondly, there are USB-CEC dongles that are not directly associated with an HDMI connector since the driver simply doesn't know. But it would be nice if the user can provide that information. I did a proof-of-concept here:
Quoted 7 lines
https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=pulse8-notifier
and it works fine. But the problem is that the connector name is currently only available on the i915 via
port_identifier(port), and that that port identifier string is not visible to the end-user. I had to try various port names ("Port A", "Port B", etc) before I hit the right one.
Is there something extra needed for the proposed approach to work in this case? Looking at the above change - since a notifier is going to be used to pass the connector info, it should work there as well. And not being a DRM specialist myself, I don't know what would be the better way (that wouldn't rely on port identifier) of pairing up a connector with an adapter.
Thank you very much for your comments and apologies for delayed response.
RFC PATCH v2 1/1 media: cec: DRM connector to CEC dev mapping
In reply to Hans VerkuilThis patch proposes to introduce explicit mapping between DRM connectors and /dev/cecX adapters.
This is achieved here by adding a new structure with connector info (card number and connector id in case of DRM connectors) to cec_adapter. That connector info is expected to be set either by a code creating an adapter or trough a CEC notifier (which is extended for that purpose).
New ioctl, exposing connector info to userland, is added to /dev/cec.
Changes since v1:
- removed the unnecessary event,
- extended cec_connctor_info to allow for various types of connectors.
--- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- drivers/gpu/drm/drm_dp_cec.c | 24 +++++++++----- drivers/gpu/drm/i915/intel_dp.c | 5 ++- drivers/gpu/drm/i915/intel_hdmi.c | 11 ++++++- drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +- drivers/media/cec/cec-adap.c | 7 +++++ drivers/media/cec/cec-api.c | 11 +++++++ drivers/media/cec/cec-core.c | 3 ++ drivers/media/cec/cec-notifier.c | 19 +++++++++++- include/drm/drm_dp_helper.h | 14 ++++----- include/media/cec-notifier.h | 31 +++++++++++++++++-- include/media/cec.h | 11 ++++++- include/uapi/linux/cec.h | 24 ++++++++++++++ 13 files changed, 139 insertions(+), 26 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +1 -1
@@ -379,7 +379,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,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->base); aconnector->mst_mgr.cbs = &dm_mst_cbs; drm_dp_mst_topology_mgr_init( &aconnector->mst_mgr,
drivers/gpu/drm/drm_dp_cec.c +16 -8
@@ -8,7 +8,9 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/slab.h> +#include <drm/drm_connector.h> #include <drm/drm_dp_helper.h> +#include <drm/drmP.h> #include <media/cec.h> /*
@@ -295,6 +297,7 @@ static void drm_dp_cec_unregister_work(struct work_struct *work)*/ void drm_dp_cec_set_edid (struct drm_dp_aux *aux,const struct edid *edid) { +struct drm_connector *connector = aux->cec.connector;u32 cec_caps =CEC_CAP_DEFAULTS |CEC_CAP_NEEDS_HPD ;unsigned int num_las =1 ;u8 cap;
@@ -344,16 +347,23 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)/* Create a new adapter */ aux->cec.adap =cec_allocate_adapter (&drm_dp_cec_adap_ops, - aux, aux->cec.name, cec_caps, + aux, connector->name, cec_caps, num_las);if (IS_ERR (aux->cec.adap)) { aux->cec.adap =NULL ; goto unlock; } -if (cec_register_adapter (aux->cec.adap, aux->cec.parent)) { +if (cec_register_adapter (aux->cec.adap, connector->dev->dev)) {cec_delete_adapter (aux->cec.adap); aux->cec.adap =NULL ; }else { +struct cec_connector_info info; + + info.type =CEC_CONNECTOR_TYPE_DRM ; + info.drm.card_no = connector->dev->primary->index; + info.drm.connector_id = connector->base.id; +cec_s_connector_info (aux->cec.adap, connector); +/* * Update the phys addr for the new CEC adapter. When called * from drm_dp_cec_register_connector() edid == NULL, so in
@@ -406,22 +416,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);/** * drm_dp_cec_register_connector() - register a new connector * @aux: DisplayPort AUX channel -* @name: name of the CEC device -* @parent: parent device +* @connector: drm connector * * A new connector was registered with associated CEC adapter name and * CEC adapter parent device. After registering the name and parent * drm_dp_cec_set_edid() is called to check if the connector supports * CEC and to register a CEC adapter if that is the case. */ -void drm_dp_cec_register_connector (struct drm_dp_aux *aux,const char *name, -struct device *parent) +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector) {WARN_ON (aux->cec.adap);if (WARN_ON (!aux->transfer))return ; - aux->cec.name = name; - aux->cec.parent = parent; + aux->cec.connector = connector;INIT_DELAYED_WORK (&aux->cec.unregister_work, drm_dp_cec_unregister_work); }
drivers/gpu/drm/i915/intel_dp.c +2 -3
@@ -5542,7 +5542,6 @@ static int intel_dp_connector_register(struct drm_connector *connector) {struct intel_dp *intel_dp =intel_attached_dp (connector); -struct drm_device *dev = connector->dev;int ret; ret =intel_connector_register (connector);
@@ -5557,8 +5556,7 @@ intel_dp_connector_register(struct drm_connector *connector) intel_dp->aux.dev = connector->kdev; ret =drm_dp_aux_register (&intel_dp->aux);if (!ret) -drm_dp_cec_register_connector (&intel_dp->aux, - connector->name, dev->dev); +drm_dp_cec_register_connector (&intel_dp->aux, connector);return ret; }
@@ -7107,3 +7105,4 @@ void intel_dp_mst_resume(struct drm_i915_private *dev_priv) } } } +
drivers/gpu/drm/i915/intel_hdmi.c +10 -1
@@ -2448,8 +2448,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, intel_hdmi->cec_notifier =cec_notifier_get_conn (dev->dev,port_identifier (port)); -if (!intel_hdmi->cec_notifier) +if (intel_hdmi->cec_notifier) { +struct cec_connector_info info; + + info.type =CEC_CONNECTOR_TYPE_DRM ; + info.drm.card_no = connector->dev->primary->index; + info.drm.connector_id = connector->base.id; +cec_notifier_set_connector_info (intel_hdmi->cec_notifier, + &info); + }else {DRM_DEBUG_KMS ("CEC notifier get failed\n" ); + } } void intel_hdmi_init(struct drm_i915_private *dev_priv,
drivers/gpu/drm/nouveau/nouveau_connector.c +1 -2
@@ -1413,8 +1413,7 @@ nouveau_connector_create(struct drm_device *dev,switch (type) {case DRM_MODE_CONNECTOR_DisplayPort:case DRM_MODE_CONNECTOR_eDP: -drm_dp_cec_register_connector (&nv_connector->aux, - connector->name, dev->dev); +drm_dp_cec_register_connector (&nv_connector->aux, connector);break ; }
drivers/media/cec/cec-adap.c +7 -0
@@ -1566,6 +1566,13 @@ void cec_s_phys_addr_from_edid(struct cec_adapter *adap, }EXPORT_SYMBOL_GPL (cec_s_phys_addr_from_edid); +void cec_s_connector_info (struct cec_adapter *adap, +struct cec_connector_info *info) +{ +memcpy (&adap->connector, info,sizeof (*info)); +} +EXPORT_SYMBOL_GPL (cec_s_connector_info); +/* * Called from either the ioctl or a driver to set the logical addresses. *
drivers/media/cec/cec-api.c +11 -0
@@ -187,6 +187,14 @@ static long cec_adap_s_log_addrs(struct cec_adapter *adap, struct cec_fh *fh,return 0 ; } +static long cec_adap_g_connector_info (struct cec_adapter *adap, +struct cec_connector_info __user *parg) +{ +if (copy_to_user (parg, &adap->connector,sizeof (adap->connector))) +return -EFAULT ; +return 0 ; +} +static long cec_transmit (struct cec_adapter *adap,struct cec_fh *fh,bool block,struct cec_msg __user *parg) {
@@ -514,6 +522,9 @@ static long cec_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)case CEC_ADAP_S_LOG_ADDRS :return cec_adap_s_log_addrs (adap, fh, block, parg); +case CEC_ADAP_G_CONNECTOR_INFO : +return cec_adap_g_connector_info (adap, parg); +case CEC_TRANSMIT :return cec_transmit (adap, fh, block, parg);
drivers/media/cec/cec-core.c +3 -0
@@ -195,6 +195,9 @@ void cec_register_cec_notifier(struct cec_adapter *adap, adap->notifier = notifier;cec_notifier_register (adap->notifier, adap, cec_cec_notify); + +cec_s_connector_info (adap, + cec_notifier_get_connector_info(notifier)); }EXPORT_SYMBOL_GPL (cec_register_cec_notifier);#endif
drivers/media/cec/cec-notifier.c +18 -1
@@ -26,6 +26,7 @@ struct cec_notifier {void (*callback)(struct cec_adapter *adap,u16 pa);u16 phys_addr; +struct cec_connector_info connector_info; };static LIST_HEAD (cec_notifiers);
@@ -51,6 +52,7 @@ struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char *conn)if (conn) n->conn =kstrdup (conn,GFP_KERNEL ); n->phys_addr =CEC_PHYS_ADDR_INVALID ; + n->connector_info.type =CEC_CONNECTOR_TYPE_NO_CONNECTOR ;mutex_init (&n->lock);kref_init (&n->kref);list_add_tail (&n->head, &cec_notifiers);
@@ -106,9 +108,24 @@ void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n, }EXPORT_SYMBOL_GPL (cec_notifier_set_phys_addr_from_edid); +void cec_notifier_set_connector_info (struct cec_notifier *n, +struct cec_connector_info *c) +{ +memcpy (&n->connector_info, c,sizeof (*c)); +} +EXPORT_SYMBOL_GPL (cec_notifier_set_connector_info); + +struct cec_connector_info *cec_notifier_get_connector_info ( +struct cec_notifier *n) +{ +return &n->connector_info; +} +EXPORT_SYMBOL_GPL (cec_notifier_get_connector_info); +void cec_notifier_register (struct cec_notifier *n,struct cec_adapter *adap, -void (*callback)(struct cec_adapter *adap,u16 pa)) +void (*callback)(struct cec_adapter *adap, +u16 pa)) {kref_get (&n->kref);mutex_lock (&n->lock);
include/drm/drm_dp_helper.h +7 -7
@@ -1208,6 +1208,7 @@ struct drm_dp_aux_msg {struct cec_adapter ;struct edid ; +struct drm_connector ;/** * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
@@ -1220,8 +1221,7 @@ struct edid;struct drm_dp_aux_cec {struct mutex lock;struct cec_adapter *adap; -const char *name; -struct device *parent; +struct drm_connector *connector;struct delayed_work unregister_work; };
@@ -1418,8 +1418,8 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)#ifdef CONFIG_DRM_DP_CEC void drm_dp_cec_irq (struct drm_dp_aux *aux); -void drm_dp_cec_register_connector (struct drm_dp_aux *aux,const char *name, -struct device *parent); +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector);void drm_dp_cec_unregister_connector (struct drm_dp_aux *aux);void drm_dp_cec_set_edid (struct drm_dp_aux *aux,const struct edid *edid);void drm_dp_cec_unset_edid (struct drm_dp_aux *aux);
@@ -1428,9 +1428,9 @@ static inline void drm_dp_cec_irq(struct drm_dp_aux *aux) { } -static inline void drm_dp_cec_register_connector (struct drm_dp_aux *aux, -const char *name, -struct device *parent) +static inline +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector) { }
include/media/cec-notifier.h +29 -2
@@ -63,6 +63,21 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa);void cec_notifier_set_phys_addr_from_edid (struct cec_notifier *n,const struct edid *edid); +/** +* cec_notifier_get_connector_info - get connector info associatied with +* a notifier +* @n: the CEC notifier +*/ +struct cec_connector_info *cec_notifier_get_connector_info ( +struct cec_notifier *n); + +/** +* cec_notifier_set_connector_info - associates connector info with a notifier +* @n: the CEC notifier +* @c: the connector info +*/ +void cec_notifier_set_connector_info (struct cec_notifier *n, +struct cec_connector_info *c);/** * cec_notifier_register - register a callback with the notifier * @n: the CEC notifier
@@ -108,9 +123,21 @@ static inline void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n, { } +const struct cec_connector_info * +cec_notifier_get_connector_info (struct cec_notifier *n) +{ +return (struct cec_connector_info *)0xdeadfeed ; +} + +static inline void cec_notifier_set_connector_info ( +struct cec_notifier *n, +struct cec_connector_info *c) +{ +} +static inline void cec_notifier_register (struct cec_notifier *n, -struct cec_adapter *adap, -void (*callback)(struct cec_adapter *adap,u16 pa)) +struct cec_adapter *adap, +void (*callback)(struct cec_adapter *adap,u16 pa)) { }
include/media/cec.h +10 -1
@@ -200,6 +200,8 @@ struct cec_adapter {u32 sequence;char input_phys[32 ]; + +struct cec_connector_info connector; };static inline void *cec_get_drvdata (const struct cec_adapter *adap)
@@ -247,9 +249,11 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block);void cec_s_phys_addr_from_edid (struct cec_adapter *adap,const struct edid *edid); +void cec_s_connector_info (struct cec_adapter *adap, +struct cec_connector_info *connector); +int cec_transmit_msg (struct cec_adapter *adap,struct cec_msg *msg,bool block); -/* Called by the adapter */ void cec_transmit_done_ts(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt, u8 nack_cnt, u8 low_drive_cnt,
@@ -357,6 +361,11 @@ static inline void cec_s_phys_addr_from_edid(struct cec_adapter *adap, { } +void cec_s_connector_info (struct cec_adapter *adap,int card_no, +u32 connector_id) +{ +} +static inline u16 cec_get_edid_phys_addr (const u8 *edid,unsigned int size,unsigned int *offset) {
include/uapi/linux/cec.h +24 -0
@@ -411,6 +411,27 @@ struct cec_event_lost_msgs {__u32 lost_msgs; }; +/** +* struct cec_event_connector - tells if and which connector is associated +* with the CEC adapter. +* @card_no: drm card number, -1 if no connector +* @connector_id: drm connector id. +*/ +struct cec_drm_connector_info { +int card_no; +__u32 connector_id; +}; + +#define CEC_CONNECTOR_TYPE_NO_CONNECTOR 0 +#define CEC_CONNECTOR_TYPE_DRM 1 +struct cec_connector_info { +int type; +union { +struct cec_drm_connector_info drm; +__u32 raw[16 ]; + }; +}; +/** * struct cec_event - CEC event structure * @ts: the timestamp of when the event was sent.
@@ -475,6 +496,9 @@ struct cec_event {#define CEC_G_MODE _IOR('a', 8, __u32)#define CEC_S_MODE _IOW('a', 9, __u32) +/* Gets the connector info */ +#define CEC_ADAP_G_CONNECTOR_INFO _IOR('a', 10, struct cec_connector_info) +/* * The remainder of this header defines all CEC messages and operands. * The format matters since it the cec-ctl utility parses it to generate
Signature · 1 line
2.18.1
Hi Dariusz,
This is much closer to what I am looking for, but it can be simplified a bit more:
Quoted 78 lines · Dariusz Marcinkiewicz
This patch proposes to introduce explicit mapping between DRM connectors and
/dev/cecXadapters.This is achieved here by adding a new structure with connector info (card number and connector id in case of DRM connectors) to
cec_adapter. That connector info is expected to be set either by a code creating an adapter or trough a CEC notifier (which is extended for that purpose).New ioctl, exposing connector info to userland, is added to
/dev/cec.Changes since v1:
- removed the unnecessary event,
- extendedcec_connctor_infoto allow for various types of connectors.Signed-off-by Dariusz Marcinkiewicz <[email protected]>--- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- drivers/gpu/drm/drm_dp_cec.c | 24 +++++++++----- drivers/gpu/drm/i915/intel_dp.c | 5 ++- drivers/gpu/drm/i915/intel_hdmi.c | 11 ++++++- drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +- drivers/media/cec/cec-adap.c | 7 +++++ drivers/media/cec/cec-api.c | 11 +++++++ drivers/media/cec/cec-core.c | 3 ++ drivers/media/cec/cec-notifier.c | 19 +++++++++++- include/drm/drm_dp_helper.h | 14 ++++----- include/media/cec-notifier.h | 31 +++++++++++++++++-- include/media/cec.h | 11 ++++++- include/uapi/linux/cec.h | 24 ++++++++++++++ 13 files changed, 139 insertions(+), 26 deletions(-)drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +1
-1@@ -379,7 +379,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,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->base); aconnector->mst_mgr.cbs = &dm_mst_cbs; drm_dp_mst_topology_mgr_init( &aconnector->mst_mgr,drivers/gpu/drm/drm_dp_cec.c +4
-2@@ -8,7 +8,9 @@#include <linux/kernel.h> #include <linux/module.h> #include <linux/slab.h> +#include <drm/drm_connector.h> #include <drm/drm_dp_helper.h> +#include <drm/drmP.h> #include <media/cec.h> /* @@ -295,6 +297,7 @@ static void drm_dp_cec_unregister_work(struct work_struct *work)*/ void drm_dp_cec_set_edid (struct drm_dp_aux *aux,const struct edid *edid) { +struct drm_connector *connector = aux->cec.connector;u32 cec_caps =CEC_CAP_DEFAULTS |CEC_CAP_NEEDS_HPD ;unsigned int num_las =1 ;u8 cap;@@ -344,16 +347,23 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)/* Create a new adapter */ aux->cec.adap =cec_allocate_adapter (&drm_dp_cec_adap_ops, - aux, aux->cec.name, cec_caps, + aux, connector->name, cec_caps, num_las);if (IS_ERR (aux->cec.adap)) { aux->cec.adap =NULL ; goto unlock; } -if (cec_register_adapter (aux->cec.adap, aux->cec.parent)) {
drivers/gpu/drm/drm_dp_cec.c void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid) · line 365
Quoted 10 lines
drivers/gpu/drm/drm_dp_cec.c +7
-0+if (cec_register_adapter (aux->cec.adap, connector->dev->dev)) {cec_delete_adapter (aux->cec.adap); aux->cec.adap =NULL ; }else { +struct cec_connector_info info; + + info.type =CEC_CONNECTOR_TYPE_DRM ; + info.drm.card_no = connector->dev->primary->index; + info.drm.connector_id = connector->base.id; +cec_s_connector_info (aux->cec.adap, connector);
Don't add this new function, just pass the information as an argument to cec_allocate_adapter. That way the adapter information is correct before the cec device is registered.
If there is no connector information, then just pass NULL to cec_allocate_adapter().
Quoted 65 lines
drivers/gpu/drm/drm_dp_cec.c +5
-6+/* * Update the phys addr for the new CEC adapter. When called * from drm_dp_cec_register_connector() edid == NULL, so in @@ -406,22 +416,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);/** * drm_dp_cec_register_connector() - register a new connector * @aux: DisplayPort AUX channel -* @name: name of the CEC device -* @parent: parent device +* @connector: drm connector * * A new connector was registered with associated CEC adapter name and * CEC adapter parent device. After registering the name and parent * drm_dp_cec_set_edid() is called to check if the connector supports * CEC and to register a CEC adapter if that is the case. */ -void drm_dp_cec_register_connector (struct drm_dp_aux *aux,const char *name, -struct device *parent) +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector) {WARN_ON (aux->cec.adap);if (WARN_ON (!aux->transfer))return ; - aux->cec.name = name; - aux->cec.parent = parent; + aux->cec.connector = connector;INIT_DELAYED_WORK (&aux->cec.unregister_work, drm_dp_cec_unregister_work); }drivers/gpu/drm/i915/intel_dp.c +2
-3@@ -5542,7 +5542,6 @@ static int intel_dp_connector_register(struct drm_connector *connector) {struct intel_dp *intel_dp =intel_attached_dp (connector); -struct drm_device *dev = connector->dev;int ret; ret =intel_connector_register (connector);@@ -5557,8 +5556,7 @@ intel_dp_connector_register(struct drm_connector *connector) intel_dp->aux.dev = connector->kdev; ret =drm_dp_aux_register (&intel_dp->aux);if (!ret) -drm_dp_cec_register_connector (&intel_dp->aux, - connector->name, dev->dev); +drm_dp_cec_register_connector (&intel_dp->aux, connector);return ret; }@@ -7107,3 +7105,4 @@ void intel_dp_mst_resume(struct drm_i915_private *dev_priv) } } } +drivers/gpu/drm/i915/intel_hdmi.c +0
-0@@ -2448,8 +2448,17 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, intel_hdmi->cec_notifier = cec_notifier_get_conn(dev->dev,
drivers/gpu/drm/i915/intel_hdmi.c void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, · line 2458
Quoted 10 lines
drivers/gpu/drm/i915/intel_hdmi.c +8
-1port_identifier(port)); -if (!intel_hdmi->cec_notifier) +if (intel_hdmi->cec_notifier) { +struct cec_connector_info info; + + info.type =CEC_CONNECTOR_TYPE_DRM ; + info.drm.card_no = connector->dev->primary->index; + info.drm.connector_id = connector->base.id; +cec_notifier_set_connector_info (intel_hdmi->cec_notifier, + &info);
Same here: the connector info should be passed as an argument to cec_notifier_get_conn() and stored in the cec_notifier struct.
The CEC driver calls cec_notifier_get() and can then pass the connector_info to the cec_allocate_adapter() call.
Quoted 295 lines
drivers/gpu/drm/i915/intel_hdmi.c +2
-0+ }else {DRM_DEBUG_KMS ("CEC notifier get failed\n" ); + } } void intel_hdmi_init(struct drm_i915_private *dev_priv,drivers/gpu/drm/nouveau/nouveau_connector.c +1
-2@@ -1413,8 +1413,7 @@ nouveau_connector_create(struct drm_device *dev,switch (type) {case DRM_MODE_CONNECTOR_DisplayPort:case DRM_MODE_CONNECTOR_eDP: -drm_dp_cec_register_connector (&nv_connector->aux, - connector->name, dev->dev); +drm_dp_cec_register_connector (&nv_connector->aux, connector);break ; }drivers/media/cec/cec-adap.c +7
-0@@ -1566,6 +1566,13 @@ void cec_s_phys_addr_from_edid(struct cec_adapter *adap, }EXPORT_SYMBOL_GPL (cec_s_phys_addr_from_edid); +void cec_s_connector_info (struct cec_adapter *adap, +struct cec_connector_info *info) +{ +memcpy (&adap->connector, info,sizeof (*info)); +} +EXPORT_SYMBOL_GPL (cec_s_connector_info); +/* * Called from either the ioctl or a driver to set the logical addresses. * drivers/media/cec/cec-api.c +11
-0@@ -187,6 +187,14 @@ static long cec_adap_s_log_addrs(struct cec_adapter *adap, struct cec_fh *fh,return 0 ; } +static long cec_adap_g_connector_info (struct cec_adapter *adap, +struct cec_connector_info __user *parg) +{ +if (copy_to_user (parg, &adap->connector,sizeof (adap->connector))) +return -EFAULT ; +return 0 ; +} +static long cec_transmit (struct cec_adapter *adap,struct cec_fh *fh,bool block,struct cec_msg __user *parg) {@@ -514,6 +522,9 @@ static long cec_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)case CEC_ADAP_S_LOG_ADDRS :return cec_adap_s_log_addrs (adap, fh, block, parg); +case CEC_ADAP_G_CONNECTOR_INFO : +return cec_adap_g_connector_info (adap, parg); +case CEC_TRANSMIT :return cec_transmit (adap, fh, block, parg);drivers/media/cec/cec-core.c +3
-0@@ -195,6 +195,9 @@ void cec_register_cec_notifier(struct cec_adapter *adap, adap->notifier = notifier;cec_notifier_register (adap->notifier, adap, cec_cec_notify); + +cec_s_connector_info (adap, + cec_notifier_get_connector_info(notifier)); }EXPORT_SYMBOL_GPL (cec_register_cec_notifier);#endif drivers/media/cec/cec-notifier.c +18
-1@@ -26,6 +26,7 @@ struct cec_notifier {void (*callback)(struct cec_adapter *adap,u16 pa);u16 phys_addr; +struct cec_connector_info connector_info; };static LIST_HEAD (cec_notifiers);@@ -51,6 +52,7 @@ struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char *conn)if (conn) n->conn =kstrdup (conn,GFP_KERNEL ); n->phys_addr =CEC_PHYS_ADDR_INVALID ; + n->connector_info.type =CEC_CONNECTOR_TYPE_NO_CONNECTOR ;mutex_init (&n->lock);kref_init (&n->kref);list_add_tail (&n->head, &cec_notifiers);@@ -106,9 +108,24 @@ void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n, }EXPORT_SYMBOL_GPL (cec_notifier_set_phys_addr_from_edid); +void cec_notifier_set_connector_info (struct cec_notifier *n, +struct cec_connector_info *c) +{ +memcpy (&n->connector_info, c,sizeof (*c)); +} +EXPORT_SYMBOL_GPL (cec_notifier_set_connector_info); + +struct cec_connector_info *cec_notifier_get_connector_info ( +struct cec_notifier *n) +{ +return &n->connector_info; +} +EXPORT_SYMBOL_GPL (cec_notifier_get_connector_info); +void cec_notifier_register (struct cec_notifier *n,struct cec_adapter *adap, -void (*callback)(struct cec_adapter *adap,u16 pa)) +void (*callback)(struct cec_adapter *adap, +u16 pa)) {kref_get (&n->kref);mutex_lock (&n->lock);include/drm/drm_dp_helper.h +7
-7@@ -1208,6 +1208,7 @@ struct drm_dp_aux_msg {struct cec_adapter ;struct edid ; +struct drm_connector ;/** * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX @@ -1220,8 +1221,7 @@ struct edid;struct drm_dp_aux_cec {struct mutex lock;struct cec_adapter *adap; -const char *name; -struct device *parent; +struct drm_connector *connector;struct delayed_work unregister_work; };@@ -1418,8 +1418,8 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)#ifdef CONFIG_DRM_DP_CEC void drm_dp_cec_irq (struct drm_dp_aux *aux); -void drm_dp_cec_register_connector (struct drm_dp_aux *aux,const char *name, -struct device *parent); +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector);void drm_dp_cec_unregister_connector (struct drm_dp_aux *aux);void drm_dp_cec_set_edid (struct drm_dp_aux *aux,const struct edid *edid);void drm_dp_cec_unset_edid (struct drm_dp_aux *aux);@@ -1428,9 +1428,9 @@ static inline void drm_dp_cec_irq(struct drm_dp_aux *aux) { } -static inline void drm_dp_cec_register_connector (struct drm_dp_aux *aux, -const char *name, -struct device *parent) +static inline +void drm_dp_cec_register_connector (struct drm_dp_aux *aux, +struct drm_connector *connector) { }include/media/cec-notifier.h +29
-2@@ -63,6 +63,21 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa);void cec_notifier_set_phys_addr_from_edid (struct cec_notifier *n,const struct edid *edid); +/** +* cec_notifier_get_connector_info - get connector info associatied with +* a notifier +* @n: the CEC notifier +*/ +struct cec_connector_info *cec_notifier_get_connector_info ( +struct cec_notifier *n); + +/** +* cec_notifier_set_connector_info - associates connector info with a notifier +* @n: the CEC notifier +* @c: the connector info +*/ +void cec_notifier_set_connector_info (struct cec_notifier *n, +struct cec_connector_info *c);/** * cec_notifier_register - register a callback with the notifier * @n: the CEC notifier @@ -108,9 +123,21 @@ static inline void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n, { } +const struct cec_connector_info * +cec_notifier_get_connector_info (struct cec_notifier *n) +{ +return (struct cec_connector_info *)0xdeadfeed ; +} + +static inline void cec_notifier_set_connector_info ( +struct cec_notifier *n, +struct cec_connector_info *c) +{ +} +static inline void cec_notifier_register (struct cec_notifier *n, -struct cec_adapter *adap, -void (*callback)(struct cec_adapter *adap,u16 pa)) +struct cec_adapter *adap, +void (*callback)(struct cec_adapter *adap,u16 pa)) { }include/media/cec.h +10
-1@@ -200,6 +200,8 @@ struct cec_adapter {u32 sequence;char input_phys[32 ]; + +struct cec_connector_info connector; };static inline void *cec_get_drvdata (const struct cec_adapter *adap)@@ -247,9 +249,11 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block);void cec_s_phys_addr_from_edid (struct cec_adapter *adap,const struct edid *edid); +void cec_s_connector_info (struct cec_adapter *adap, +struct cec_connector_info *connector); +int cec_transmit_msg (struct cec_adapter *adap,struct cec_msg *msg,bool block); -/* Called by the adapter */ void cec_transmit_done_ts(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt, u8 nack_cnt, u8 low_drive_cnt,@@ -357,6 +361,11 @@ static inline void cec_s_phys_addr_from_edid(struct cec_adapter *adap, { } +void cec_s_connector_info (struct cec_adapter *adap,int card_no, +u32 connector_id) +{ +} +static inline u16 cec_get_edid_phys_addr (const u8 *edid,unsigned int size,unsigned int *offset) {include/uapi/linux/cec.h +21
-0@@ -411,6 +411,27 @@ struct cec_event_lost_msgs {__u32 lost_msgs; }; +/** +* struct cec_event_connector - tells if and which connector is associated +* with the CEC adapter. +* @card_no: drm card number, -1 if no connector +* @connector_id: drm connector id. +*/ +struct cec_drm_connector_info { +int card_no; +__u32 connector_id; +}; + +#define CEC_CONNECTOR_TYPE_NO_CONNECTOR 0 +#define CEC_CONNECTOR_TYPE_DRM 1 +struct cec_connector_info { +int type; +union { +struct cec_drm_connector_info drm; +__u32 raw[16 ]; + }; +}; +/** * struct cec_event - CEC event structure * @ts: the timestamp of when the event was sent.
include/uapi/linux/cec.h struct cec_event { · line 504
Quoted 9 lines
include/uapi/linux/cec.h +3
-0@@ -475,6 +496,9 @@ struct cec_event {#define CEC_G_MODE _IOR('a', 8, __u32)#define CEC_S_MODE _IOW('a', 9, __u32) +/* Gets the connector info */ +#define CEC_ADAP_G_CONNECTOR_INFO _IOR('a', 10, struct cec_connector_info) +/* * The remainder of this header defines all CEC messages and operands. * The format matters since it the cec-ctl utility parses it to generate
Regards,
Hans