<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 12/12/2018 4:34 PM, C, Ramalingam
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:89a723a1-6649-4b93-8b3d-004cb99350a7@intel.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <p><br>
      </p>
      <div class="moz-cite-prefix">On 12/12/2018 4:08 PM, Daniel Vetter
        wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:20181212103802.GU21184@phenom.ffwll.local">
        <pre class="moz-quote-pre" wrap="">On Wed, Dec 12, 2018 at 02:28:29PM +0530, C, Ramalingam wrote:
</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">On 12/7/2018 7:59 PM, Daniel Vetter wrote:
</pre>
          <blockquote type="cite">
            <pre class="moz-quote-pre" wrap="">On Fri, Dec 07, 2018 at 11:22:44AM +0530, C, Ramalingam wrote:
</pre>
            <blockquote type="cite">
              <pre class="moz-quote-pre" wrap="">On 12/6/2018 3:53 PM, Daniel Vetter wrote:
</pre>
              <blockquote type="cite">
                <pre class="moz-quote-pre" wrap="">On Tue, Nov 27, 2018 at 04:13:03PM +0530, Ramalingam C wrote:
</pre>
                <blockquote type="cite">
                  <pre class="moz-quote-pre" wrap="">+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);
</pre>
                </blockquote>
                <pre class="moz-quote-pre" wrap="">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.
</pre>
              </blockquote>
              <pre class="moz-quote-pre" wrap="">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.
</pre>
            </blockquote>
            <pre class="moz-quote-pre" wrap="">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.
</pre>
          </blockquote>
          <pre class="moz-quote-pre" wrap="">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.?
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">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.</pre>
      </blockquote>
      <pre>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.</pre>
    </blockquote>
    <pre>Summarizing the #intel-gfx IRC discussion:</pre>
    <pre>As i915 load and unload  are already asymetric, there is no harm in moving
the drm_atomic_helper_shutdown() into the master_unbind(). So going
ahead with daniel's suggestion.

-Ram
</pre>
    <blockquote type="cite"
      cite="mid:89a723a1-6649-4b93-8b3d-004cb99350a7@intel.com">
      <pre>

-Ram
</pre>
      <blockquote type="cite"
        cite="mid:20181212103802.GU21184@phenom.ffwll.local">
        <pre class="moz-quote-pre" wrap="">-Daniel
</pre>
      </blockquote>
    </blockquote>
  </body>
</html>