On Thursday 22 February 2018 08:09 PM, Sean Paul wrote:
On Wed, Feb 14, 2018 at 07:43:37PM +0530, Ramalingam C wrote:
Each HDCP authentication, could take upto 5.1Sec, based on the
downstream HDCP topology.
Hence to avoid this much delay in the atomic_commit path, this patch
schedules the HDCP authentication into a asynchronous work.
This keeps the UI active, by enabling the flips in parallel to
HDCP auth.
Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_hdcp.c | 36 ++++++++++++++++++++++--------------
3 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 30e38cbeface..048d60b5143b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15317,6 +15317,7 @@ static void intel_hpd_poll_fini(struct drm_device *dev)
if (connector->hdcp_shim) {
cancel_delayed_work_sync(&connector->hdcp_check_work);
cancel_work_sync(&connector->hdcp_prop_work);
+ cancel_work_sync(&connector->hdcp_enable_work);
}
}
drm_connector_list_iter_end(&conn_iter);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 898064e8bea7..7b9e5f70826f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -412,6 +412,7 @@ struct intel_connector {
uint64_t hdcp_value; /* protected by hdcp_mutex */
struct delayed_work hdcp_check_work;
struct work_struct hdcp_prop_work;
+ struct work_struct hdcp_enable_work;
};
struct intel_digital_connector_state {
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
index 14ca5d3057a7..e03bd376d92c 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -600,8 +600,14 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
for (i = 0; i < tries; i++) {
ret = intel_hdcp_auth(conn_to_dig_port(connector),
connector->hdcp_shim);
- if (!ret)
+ if (!ret) {
+ connector->hdcp_value =
+ DRM_MODE_CONTENT_PROTECTION_ENABLED;
+ schedule_work(&connector->hdcp_prop_work);
+ schedule_delayed_work(&connector->hdcp_check_work,
+ DRM_HDCP_CHECK_PERIOD_MS);
return 0;
+ }
DRM_DEBUG_KMS("HDCP Auth failure (%d)\n", ret);
@@ -613,6 +619,17 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
return ret;
}
+static void intel_hdcp_enable_work(struct work_struct *work)
+{
+ struct intel_connector *connector = container_of(work,
+ struct intel_connector,
+ hdcp_enable_work);
+
+ mutex_lock(&connector->hdcp_mutex);
+ _intel_hdcp_enable(connector);
+ mutex_unlock(&connector->hdcp_mutex);
+}
+
static void intel_hdcp_check_work(struct work_struct *work)
{
struct intel_connector *connector = container_of(to_delayed_work(work),
@@ -669,29 +686,20 @@ int intel_hdcp_init(struct intel_connector *connector,
mutex_init(&connector->hdcp_mutex);
INIT_DELAYED_WORK(&connector->hdcp_check_work, intel_hdcp_check_work);
INIT_WORK(&connector->hdcp_prop_work, intel_hdcp_prop_work);
+ INIT_WORK(&connector->hdcp_enable_work, intel_hdcp_enable_work);
return 0;
}
int intel_hdcp_enable(struct intel_connector *connector)
{
- int ret;
-
if (!connector->hdcp_shim)
return -ENOENT;
mutex_lock(&connector->hdcp_mutex);
Why do you need to hold this lock to schedule the worker?
-
- ret = _intel_hdcp_enable(connector);
- if (ret)
- goto out;
-
- connector->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
- schedule_work(&connector->hdcp_prop_work);
- schedule_delayed_work(&connector->hdcp_check_work,
- DRM_HDCP_CHECK_PERIOD_MS);
-out:
+ schedule_work(&connector->hdcp_enable_work);
How is this worker cancelled when appropriate (enable/disable, hotplug, etc)?
At present enable flow also non-interuptible on hotplug. disable is not
even is possible along with enable. correct me if i am wrong.
Since enable flow is blocking the modeset(atomic) also, I am afraid we
need to adopt a worker for
enable path for facilitate the hotplug processing.
And in case of worker implementation need to check the connector status
and hdcp preferred state before entering into any wait for msg.
Sleep should be selectively interruptible. every reauth should be
checking the above conditions. I feel this is do-able.
This patch also not doing all of the above. If you think above prob
statement needs to be addressed i will work on that.
I am looking for your suggestion. Thanks
I'm not convinced the extra complexity/worker is worth it. While it's possible
it _could_ take 5 seconds, however I've never experienced any lengthy delays in
practice.
Because hdcp was passing on all setup, you were done in less than a Sec.
That is the best case scenario. I wish every setup falls into it :)
Due to a buggy repeater or a buggy receiver connected to a functional
repeater,
if the repeater authentication is delayed/failed, flip will be blocked
for ~5Secs x <no of auth_tries>
All these possibilities can be avoided, if we can handle the complexity
attached to worker
scheduling and canceling. Please share your take on this.
IMO, it's more important to remove the modeset requirement (we've
fixed this in CrOS already) than it is to do HDCP asynchronously.
Agreed on modeset removal. We need to do this on priority.
--Ram
Sean
mutex_unlock(&connector->hdcp_mutex);
- return ret;
+
+ return 0;
}
int intel_hdcp_disable(struct intel_connector *connector)
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
