Re: [PATCH v8 05/35] drm/i915: MEI interface definition

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/12/2018 4:08 PM, Daniel Vetter wrote:
On Wed, Dec 12, 2018 at 02:28:29PM +0530, C, Ramalingam wrote:
On 12/7/2018 7:59 PM, Daniel Vetter wrote:
On Fri, Dec 07, 2018 at 11:22:44AM +0530, C, Ramalingam wrote:
On 12/6/2018 3:53 PM, Daniel Vetter wrote:
On Tue, Nov 27, 2018 at 04:13:03PM +0530, Ramalingam C wrote:
+static void i915_hdcp_component_master_unbind(struct device *dev)
+{
+	struct drm_i915_private *dev_priv = kdev_to_i915(dev);
+
+	intel_connectors_hdcp_disable(dev_priv);
Why this code? Once we've unregistered the driver, we should have shut
down the hardware completely. Which should have shut down all the hdcp
users too.
This unbind might be triggered either due to master_del or component_del.
if its triggered from mei through component_del, then before teardown of
the i/f in component_unbind(), disable the ongoing HDCP session through
hdcp_disable() for each connectors.
I looked at your v7 component code again. I think if we put the
drm_atomic_helper_shutdown() call into master_unbind, then we'll have taken care
of that. Essentially what you're doing here is open-coding part of that
function. Better to just move the existing call to the right place.

And shutting down the entire hw is kinda what master_unbind should be
doing I think. We might also need to move the general hw quiescent into
master_unbind, that should work too.
Need some more information on this. As per v7 on master_unbind will invoke
i915_unload_head that is i915_driver_unregister(dev_priv);
Similarly master_bind will call the load_tail that is i915_driver_register(dev_priv);

We are not initializing/shutting the HW in master_bind/unbind .
But this comment is contradicting with current approach. Could you please elaborate.?
So my understanding is that we need to shut down all hdcp usage before
master_unbind completes, because otherwise we'll blow up: The mei_hdcp
component is gone already.

Now the other case for master_unbind is that the i915 pci device is
unloaded, and in that case again I think it makes sense to shut down the
hardware in master_unbind.

Now when I looked at v7, right after the component_unbind code in the i915
unload sequences comes the function calls to shut down the hardware. I
think it would make sense to them (or at least the
drm_atomic_helper_shutdown() call) into master_unbind.

This is somewhat asymetric, but that's ok: Load path doesn't enable
anything, we just keep the hardware as-is (to be able to support
flicker-free boôt-up). First modest is done by usersapce. Exception is if
you have fbcon enabled (and not use the fastboot patches that Hans just
merged), in that case the kernel will do the first modeset when we
regiseter the fbdev device.

Anyway, moving the drm_atomic_helper_shutdown() into the master_unbind
function in v7 should take care of disabling all outputs, and hence also
disabling all usage of hdcp component.
But in that case master_bind should do the reverse of the drm_atomic_helper_shutdown()right?
else lets say mei_hdcp is removed, master_unbind triggered and hence all hw is shutdown.
And then mei_hdcp is loaded so master_bind should initialize the hw right? Which is not the scenario right now.

-Ram
-Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux