* [PATCH v7 1/3] drm_bridge: register content protect property @ 2022-11-23 10:05 Hsin-Yi Wang 2022-11-23 10:05 ` [PATCH v7 2/3] drm/bridge: anx7625: " Hsin-Yi Wang 2022-11-23 10:05 ` [PATCH v7 3/3] drm/bridge: it6505: handle HDCP request Hsin-Yi Wang 0 siblings, 2 replies; 5+ messages in thread From: Hsin-Yi Wang @ 2022-11-23 10:05 UTC (permalink / raw) To: Sean Paul, Douglas Anderson, Robert Foss Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Daniel Vetter, dri-devel, linux-kernel, Allen Chen, David Airlie Some bridges are able to update HDCP status from userspace request if they support HDCP. HDCP property is the same as other connector properties that needs to be created after the connecter is initialized and before the connector is registered. If there exists a bridge that supports HDCP, add the property to the bridge connector. Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> Reviewed-by: Sean Paul <seanpaul@chromium.org> --- v6->v7: remove reported by bot tag --- drivers/gpu/drm/drm_bridge_connector.c | 9 +++++++++ include/drm/drm_bridge.h | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 1c7d936523df..4147c6240110 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -7,6 +7,7 @@ #include <linux/module.h> #include <linux/slab.h> +#include <drm/display/drm_hdcp_helper.h> #include <drm/drm_atomic_state_helper.h> #include <drm/drm_bridge.h> #include <drm/drm_bridge_connector.h> @@ -333,6 +334,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, struct i2c_adapter *ddc = NULL; struct drm_bridge *bridge, *panel_bridge = NULL; int connector_type; + bool support_hdcp = false; bridge_connector = kzalloc(sizeof(*bridge_connector), GFP_KERNEL); if (!bridge_connector) @@ -376,6 +378,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (drm_bridge_is_panel(bridge)) panel_bridge = bridge; + + if (bridge->support_hdcp) + support_hdcp = true; } if (connector_type == DRM_MODE_CONNECTOR_Unknown) { @@ -398,6 +403,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (panel_bridge) drm_panel_bridge_set_orientation(connector, panel_bridge); + if (support_hdcp && IS_REACHABLE(CONFIG_DRM_DISPLAY_HELPER) && + IS_ENABLED(CONFIG_DRM_DISPLAY_HDCP_HELPER)) + drm_connector_attach_content_protection_property(connector, true); + return connector; } EXPORT_SYMBOL_GPL(drm_bridge_connector_init); diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 6b65b0dfb4fb..1d2ab70f3436 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -768,6 +768,10 @@ struct drm_bridge { * modes. */ bool interlace_allowed; + /** + * @support_hdcp: Indicate that the bridge supports HDCP. + */ + bool support_hdcp; /** * @ddc: Associated I2C adapter for DDC access, if any. */ -- 2.38.1.584.g0f3c55d4c2-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v7 2/3] drm/bridge: anx7625: register content protect property 2022-11-23 10:05 [PATCH v7 1/3] drm_bridge: register content protect property Hsin-Yi Wang @ 2022-11-23 10:05 ` Hsin-Yi Wang 2022-11-23 10:05 ` [PATCH v7 3/3] drm/bridge: it6505: handle HDCP request Hsin-Yi Wang 1 sibling, 0 replies; 5+ messages in thread From: Hsin-Yi Wang @ 2022-11-23 10:05 UTC (permalink / raw) To: Sean Paul, Douglas Anderson, Robert Foss Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Daniel Vetter, dri-devel, linux-kernel, Allen Chen, David Airlie Set support_hdcp so the connector can register content protect proterty when it's initializing. Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> Reviewed-by: Sean Paul <seanpaul@chromium.org> --- v6->v7: no change --- drivers/gpu/drm/bridge/analogix/anx7625.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index b0ff1ecb80a5..0636ac59c739 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -2680,6 +2680,7 @@ static int anx7625_i2c_probe(struct i2c_client *client, platform->bridge.type = platform->pdata.panel_bridge ? DRM_MODE_CONNECTOR_eDP : DRM_MODE_CONNECTOR_DisplayPort; + platform->bridge.support_hdcp = true; drm_bridge_add(&platform->bridge); -- 2.38.1.584.g0f3c55d4c2-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v7 3/3] drm/bridge: it6505: handle HDCP request 2022-11-23 10:05 [PATCH v7 1/3] drm_bridge: register content protect property Hsin-Yi Wang 2022-11-23 10:05 ` [PATCH v7 2/3] drm/bridge: anx7625: " Hsin-Yi Wang @ 2022-11-23 10:05 ` Hsin-Yi Wang 2022-11-23 20:16 ` Sean Paul 1 sibling, 1 reply; 5+ messages in thread From: Hsin-Yi Wang @ 2022-11-23 10:05 UTC (permalink / raw) To: Sean Paul, Douglas Anderson, Robert Foss Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Daniel Vetter, dri-devel, linux-kernel, Allen Chen, David Airlie it6505 supports HDCP 1.3, but current implementation lacks the update of HDCP status through drm_hdcp_update_content_protection(). it6505 default enables the HDCP. Remove this and only turn on when user requests it. Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> Reviewed-by: allen chen <allen.chen@ite.com.tw> --- v6->v7: remove enable hdcp by default. --- drivers/gpu/drm/bridge/ite-it6505.c | 60 +++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c index 21a9b8422bda..93626698c31e 100644 --- a/drivers/gpu/drm/bridge/ite-it6505.c +++ b/drivers/gpu/drm/bridge/ite-it6505.c @@ -423,6 +423,7 @@ struct it6505 { struct extcon_dev *extcon; struct work_struct extcon_wq; int extcon_state; + struct drm_connector *connector; enum drm_connector_status connector_status; enum link_train_status link_state; struct work_struct link_works; @@ -2159,9 +2160,6 @@ static void it6505_link_train_ok(struct it6505 *it6505) DRM_DEV_DEBUG_DRIVER(dev, "Enable audio!"); it6505_enable_audio(it6505); } - - if (it6505->hdcp_desired) - it6505_start_hdcp(it6505); } static void it6505_link_step_train_process(struct it6505 *it6505) @@ -2399,6 +2397,14 @@ static void it6505_irq_hdcp_done(struct it6505 *it6505) DRM_DEV_DEBUG_DRIVER(dev, "hdcp done interrupt"); it6505->hdcp_status = HDCP_AUTH_DONE; + if (it6505->connector) { + struct drm_device *drm_dev = it6505->connector->dev; + + drm_modeset_lock(&drm_dev->mode_config.connection_mutex, NULL); + drm_hdcp_update_content_protection(it6505->connector, + DRM_MODE_CONTENT_PROTECTION_ENABLED); + drm_modeset_unlock(&drm_dev->mode_config.connection_mutex); + } it6505_show_hdcp_info(it6505); } @@ -2931,6 +2937,7 @@ static void it6505_bridge_atomic_enable(struct drm_bridge *bridge, if (WARN_ON(!connector)) return; + it6505->connector = connector; conn_state = drm_atomic_get_new_connector_state(state, connector); if (WARN_ON(!conn_state)) @@ -2974,6 +2981,7 @@ static void it6505_bridge_atomic_disable(struct drm_bridge *bridge, DRM_DEV_DEBUG_DRIVER(dev, "start"); + it6505->connector = NULL; if (it6505->powered) { it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link, DP_SET_POWER_D3); @@ -3028,6 +3036,50 @@ static struct edid *it6505_bridge_get_edid(struct drm_bridge *bridge, return edid; } +static int it6505_connector_atomic_check(struct it6505 *it6505, + struct drm_connector_state *state) +{ + struct device *dev = &it6505->client->dev; + int cp = state->content_protection; + + DRM_DEV_DEBUG_DRIVER(dev, "hdcp connector state:%d, curr hdcp state:%d", + cp, it6505->hdcp_status); + + if (!it6505->hdcp_desired) { + DRM_DEV_DEBUG_DRIVER(dev, "sink not support hdcp"); + return 0; + } + + if (it6505->hdcp_status == HDCP_AUTH_GOING) + return -EINVAL; + + if (cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED) { + if (it6505->hdcp_status == HDCP_AUTH_DONE) + it6505_stop_hdcp(it6505); + } else if (cp == DRM_MODE_CONTENT_PROTECTION_DESIRED) { + if (it6505->hdcp_status == HDCP_AUTH_IDLE && + it6505->link_state == LINK_OK) + it6505_start_hdcp(it6505); + } else { + if (it6505->hdcp_status == HDCP_AUTH_IDLE) { + DRM_DEV_DEBUG_DRIVER(dev, "invalid to set hdcp enabled"); + return -EINVAL; + } + } + + return 0; +} + +static int it6505_bridge_atomic_check(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct it6505 *it6505 = bridge_to_it6505(bridge); + + return it6505_connector_atomic_check(it6505, conn_state); +} + static const struct drm_bridge_funcs it6505_bridge_funcs = { .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, @@ -3035,6 +3087,7 @@ static const struct drm_bridge_funcs it6505_bridge_funcs = { .attach = it6505_bridge_attach, .detach = it6505_bridge_detach, .mode_valid = it6505_bridge_mode_valid, + .atomic_check = it6505_bridge_atomic_check, .atomic_enable = it6505_bridge_atomic_enable, .atomic_disable = it6505_bridge_atomic_disable, .atomic_pre_enable = it6505_bridge_atomic_pre_enable, @@ -3354,6 +3407,7 @@ static int it6505_i2c_probe(struct i2c_client *client, it6505->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; it6505->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD; + it6505->bridge.support_hdcp = true; drm_bridge_add(&it6505->bridge); return 0; -- 2.38.1.584.g0f3c55d4c2-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v7 3/3] drm/bridge: it6505: handle HDCP request 2022-11-23 10:05 ` [PATCH v7 3/3] drm/bridge: it6505: handle HDCP request Hsin-Yi Wang @ 2022-11-23 20:16 ` Sean Paul 2022-11-24 4:53 ` Hsin-Yi Wang 0 siblings, 1 reply; 5+ messages in thread From: Sean Paul @ 2022-11-23 20:16 UTC (permalink / raw) To: Hsin-Yi Wang Cc: Sean Paul, Douglas Anderson, Robert Foss, Thomas Zimmermann, Allen Chen, linux-kernel, dri-devel On Wed, Nov 23, 2022 at 10:05:29AM +0000, Hsin-Yi Wang wrote: > it6505 supports HDCP 1.3, but current implementation lacks the update of > HDCP status through drm_hdcp_update_content_protection(). > > it6505 default enables the HDCP. Remove this and only turn on when user > requests it. > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> > Reviewed-by: allen chen <allen.chen@ite.com.tw> > --- > v6->v7: remove enable hdcp by default. > --- > drivers/gpu/drm/bridge/ite-it6505.c | 60 +++++++++++++++++++++++++++-- > 1 file changed, 57 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c > index 21a9b8422bda..93626698c31e 100644 > --- a/drivers/gpu/drm/bridge/ite-it6505.c > +++ b/drivers/gpu/drm/bridge/ite-it6505.c > @@ -423,6 +423,7 @@ struct it6505 { > struct extcon_dev *extcon; > struct work_struct extcon_wq; > int extcon_state; > + struct drm_connector *connector; > enum drm_connector_status connector_status; > enum link_train_status link_state; > struct work_struct link_works; > @@ -2159,9 +2160,6 @@ static void it6505_link_train_ok(struct it6505 *it6505) > DRM_DEV_DEBUG_DRIVER(dev, "Enable audio!"); > it6505_enable_audio(it6505); > } > - > - if (it6505->hdcp_desired) > - it6505_start_hdcp(it6505); > } > > static void it6505_link_step_train_process(struct it6505 *it6505) > @@ -2399,6 +2397,14 @@ static void it6505_irq_hdcp_done(struct it6505 *it6505) > > DRM_DEV_DEBUG_DRIVER(dev, "hdcp done interrupt"); > it6505->hdcp_status = HDCP_AUTH_DONE; > + if (it6505->connector) { > + struct drm_device *drm_dev = it6505->connector->dev; > + > + drm_modeset_lock(&drm_dev->mode_config.connection_mutex, NULL); > + drm_hdcp_update_content_protection(it6505->connector, > + DRM_MODE_CONTENT_PROTECTION_ENABLED); > + drm_modeset_unlock(&drm_dev->mode_config.connection_mutex); > + } > it6505_show_hdcp_info(it6505); > } > > @@ -2931,6 +2937,7 @@ static void it6505_bridge_atomic_enable(struct drm_bridge *bridge, > if (WARN_ON(!connector)) > return; > > + it6505->connector = connector; > conn_state = drm_atomic_get_new_connector_state(state, connector); > > if (WARN_ON(!conn_state)) > @@ -2974,6 +2981,7 @@ static void it6505_bridge_atomic_disable(struct drm_bridge *bridge, > > DRM_DEV_DEBUG_DRIVER(dev, "start"); > > + it6505->connector = NULL; > if (it6505->powered) { > it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link, > DP_SET_POWER_D3); > @@ -3028,6 +3036,50 @@ static struct edid *it6505_bridge_get_edid(struct drm_bridge *bridge, > return edid; > } > > +static int it6505_connector_atomic_check(struct it6505 *it6505, > + struct drm_connector_state *state) > +{ > + struct device *dev = &it6505->client->dev; > + int cp = state->content_protection; > + > + DRM_DEV_DEBUG_DRIVER(dev, "hdcp connector state:%d, curr hdcp state:%d", > + cp, it6505->hdcp_status); > + > + if (!it6505->hdcp_desired) { > + DRM_DEV_DEBUG_DRIVER(dev, "sink not support hdcp"); > + return 0; > + } > + > + if (it6505->hdcp_status == HDCP_AUTH_GOING) > + return -EINVAL; > + > + if (cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED) { > + if (it6505->hdcp_status == HDCP_AUTH_DONE) > + it6505_stop_hdcp(it6505); You shouldn't touch the hardware in atomic_check, this should be done in the commit. > + } else if (cp == DRM_MODE_CONTENT_PROTECTION_DESIRED) { > + if (it6505->hdcp_status == HDCP_AUTH_IDLE && > + it6505->link_state == LINK_OK) > + it6505_start_hdcp(it6505); Same here > + } else { > + if (it6505->hdcp_status == HDCP_AUTH_IDLE) { > + DRM_DEV_DEBUG_DRIVER(dev, "invalid to set hdcp enabled"); > + return -EINVAL; > + } > + } In general, I think there are a number of locking and state issues with this approach. I had pulled all of this logic into a set of helpers [1], but the patchset has gotten stale on the list. You might consider looking at patches 1-4 to see how the state and locking should be handled to avoid races. Sean [1] - https://lore.kernel.org/all/20220411204741.1074308-1-sean@poorly.run/ > + > + return 0; > +} > + > +static int it6505_bridge_atomic_check(struct drm_bridge *bridge, > + struct drm_bridge_state *bridge_state, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + struct it6505 *it6505 = bridge_to_it6505(bridge); > + > + return it6505_connector_atomic_check(it6505, conn_state); > +} > + > static const struct drm_bridge_funcs it6505_bridge_funcs = { > .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > @@ -3035,6 +3087,7 @@ static const struct drm_bridge_funcs it6505_bridge_funcs = { > .attach = it6505_bridge_attach, > .detach = it6505_bridge_detach, > .mode_valid = it6505_bridge_mode_valid, > + .atomic_check = it6505_bridge_atomic_check, > .atomic_enable = it6505_bridge_atomic_enable, > .atomic_disable = it6505_bridge_atomic_disable, > .atomic_pre_enable = it6505_bridge_atomic_pre_enable, > @@ -3354,6 +3407,7 @@ static int it6505_i2c_probe(struct i2c_client *client, > it6505->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; > it6505->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | > DRM_BRIDGE_OP_HPD; > + it6505->bridge.support_hdcp = true; > drm_bridge_add(&it6505->bridge); > > return 0; > -- > 2.38.1.584.g0f3c55d4c2-goog > -- Sean Paul, Software Engineer, Google / Chromium OS ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v7 3/3] drm/bridge: it6505: handle HDCP request 2022-11-23 20:16 ` Sean Paul @ 2022-11-24 4:53 ` Hsin-Yi Wang 0 siblings, 0 replies; 5+ messages in thread From: Hsin-Yi Wang @ 2022-11-24 4:53 UTC (permalink / raw) To: Sean Paul Cc: Sean Paul, Douglas Anderson, Robert Foss, Thomas Zimmermann, Allen Chen, linux-kernel, dri-devel On Thu, Nov 24, 2022 at 4:16 AM Sean Paul <sean@poorly.run> wrote: > > On Wed, Nov 23, 2022 at 10:05:29AM +0000, Hsin-Yi Wang wrote: > > it6505 supports HDCP 1.3, but current implementation lacks the update of > > HDCP status through drm_hdcp_update_content_protection(). > > > > it6505 default enables the HDCP. Remove this and only turn on when user > > requests it. > > > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> > > Reviewed-by: allen chen <allen.chen@ite.com.tw> > > --- > > v6->v7: remove enable hdcp by default. > > --- > > drivers/gpu/drm/bridge/ite-it6505.c | 60 +++++++++++++++++++++++++++-- > > 1 file changed, 57 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c > > index 21a9b8422bda..93626698c31e 100644 > > --- a/drivers/gpu/drm/bridge/ite-it6505.c > > +++ b/drivers/gpu/drm/bridge/ite-it6505.c > > @@ -423,6 +423,7 @@ struct it6505 { > > struct extcon_dev *extcon; > > struct work_struct extcon_wq; > > int extcon_state; > > + struct drm_connector *connector; > > enum drm_connector_status connector_status; > > enum link_train_status link_state; > > struct work_struct link_works; > > @@ -2159,9 +2160,6 @@ static void it6505_link_train_ok(struct it6505 *it6505) > > DRM_DEV_DEBUG_DRIVER(dev, "Enable audio!"); > > it6505_enable_audio(it6505); > > } > > - > > - if (it6505->hdcp_desired) > > - it6505_start_hdcp(it6505); > > } > > > > static void it6505_link_step_train_process(struct it6505 *it6505) > > @@ -2399,6 +2397,14 @@ static void it6505_irq_hdcp_done(struct it6505 *it6505) > > > > DRM_DEV_DEBUG_DRIVER(dev, "hdcp done interrupt"); > > it6505->hdcp_status = HDCP_AUTH_DONE; > > + if (it6505->connector) { > > + struct drm_device *drm_dev = it6505->connector->dev; > > + > > + drm_modeset_lock(&drm_dev->mode_config.connection_mutex, NULL); > > + drm_hdcp_update_content_protection(it6505->connector, > > + DRM_MODE_CONTENT_PROTECTION_ENABLED); > > + drm_modeset_unlock(&drm_dev->mode_config.connection_mutex); > > + } > > it6505_show_hdcp_info(it6505); > > } > > > > @@ -2931,6 +2937,7 @@ static void it6505_bridge_atomic_enable(struct drm_bridge *bridge, > > if (WARN_ON(!connector)) > > return; > > > > + it6505->connector = connector; > > conn_state = drm_atomic_get_new_connector_state(state, connector); > > > > if (WARN_ON(!conn_state)) > > @@ -2974,6 +2981,7 @@ static void it6505_bridge_atomic_disable(struct drm_bridge *bridge, > > > > DRM_DEV_DEBUG_DRIVER(dev, "start"); > > > > + it6505->connector = NULL; > > if (it6505->powered) { > > it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link, > > DP_SET_POWER_D3); > > @@ -3028,6 +3036,50 @@ static struct edid *it6505_bridge_get_edid(struct drm_bridge *bridge, > > return edid; > > } > > > > +static int it6505_connector_atomic_check(struct it6505 *it6505, > > + struct drm_connector_state *state) > > +{ > > + struct device *dev = &it6505->client->dev; > > + int cp = state->content_protection; > > + > > + DRM_DEV_DEBUG_DRIVER(dev, "hdcp connector state:%d, curr hdcp state:%d", > > + cp, it6505->hdcp_status); > > + > > + if (!it6505->hdcp_desired) { > > + DRM_DEV_DEBUG_DRIVER(dev, "sink not support hdcp"); > > + return 0; > > + } > > + > > + if (it6505->hdcp_status == HDCP_AUTH_GOING) > > + return -EINVAL; > > + > > + if (cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED) { > > + if (it6505->hdcp_status == HDCP_AUTH_DONE) > > + it6505_stop_hdcp(it6505); > > You shouldn't touch the hardware in atomic_check, this should be done in the > commit. > Since it6505 is a bridge, do you have suggested callbacks in drm_bridge_funcs similar to commit? > > + } else if (cp == DRM_MODE_CONTENT_PROTECTION_DESIRED) { > > + if (it6505->hdcp_status == HDCP_AUTH_IDLE && > > + it6505->link_state == LINK_OK) > > + it6505_start_hdcp(it6505); > > Same here > > > + } else { > > + if (it6505->hdcp_status == HDCP_AUTH_IDLE) { > > + DRM_DEV_DEBUG_DRIVER(dev, "invalid to set hdcp enabled"); > > + return -EINVAL; > > + } > > + } > > In general, I think there are a number of locking and state issues with this > approach. I had pulled all of this logic into a set of helpers [1], but the > patchset has gotten stale on the list. You might consider looking at patches 1-4 > to see how the state and locking should be handled to avoid races. > After checking the series, all the hdcp controls are done in dp controller instead of bridges. Is it proper for the bridge to update the HDCP state? - If it's not, then this series might be going in the wrong direction. - If it is, since the current it6505 enables HDCP in the hardware (it6505_start_hdcp() is called ) all the time, can we just update the state without turning it off? > Sean > > [1] - https://lore.kernel.org/all/20220411204741.1074308-1-sean@poorly.run/ > > > + > > + return 0; > > +} > > + > > +static int it6505_bridge_atomic_check(struct drm_bridge *bridge, > > + struct drm_bridge_state *bridge_state, > > + struct drm_crtc_state *crtc_state, > > + struct drm_connector_state *conn_state) > > +{ > > + struct it6505 *it6505 = bridge_to_it6505(bridge); > > + > > + return it6505_connector_atomic_check(it6505, conn_state); > > +} > > + > > static const struct drm_bridge_funcs it6505_bridge_funcs = { > > .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > > .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > > @@ -3035,6 +3087,7 @@ static const struct drm_bridge_funcs it6505_bridge_funcs = { > > .attach = it6505_bridge_attach, > > .detach = it6505_bridge_detach, > > .mode_valid = it6505_bridge_mode_valid, > > + .atomic_check = it6505_bridge_atomic_check, > > .atomic_enable = it6505_bridge_atomic_enable, > > .atomic_disable = it6505_bridge_atomic_disable, > > .atomic_pre_enable = it6505_bridge_atomic_pre_enable, > > @@ -3354,6 +3407,7 @@ static int it6505_i2c_probe(struct i2c_client *client, > > it6505->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; > > it6505->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | > > DRM_BRIDGE_OP_HPD; > > + it6505->bridge.support_hdcp = true; > > drm_bridge_add(&it6505->bridge); > > > > return 0; > > -- > > 2.38.1.584.g0f3c55d4c2-goog > > > > -- > Sean Paul, Software Engineer, Google / Chromium OS ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-11-24 4:53 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-23 10:05 [PATCH v7 1/3] drm_bridge: register content protect property Hsin-Yi Wang 2022-11-23 10:05 ` [PATCH v7 2/3] drm/bridge: anx7625: " Hsin-Yi Wang 2022-11-23 10:05 ` [PATCH v7 3/3] drm/bridge: it6505: handle HDCP request Hsin-Yi Wang 2022-11-23 20:16 ` Sean Paul 2022-11-24 4:53 ` Hsin-Yi Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).