Re: [Intel-gfx] [RFC PATCH 0/6] drm/i915: Implement HDCP

2017-12-05 Thread Sean Paul 
On Tue, Dec 5, 2017 at 8:45 AM, Ville Syrjälä
 wrote:
> On Thu, Nov 30, 2017 at 08:50:38AM +0100, Daniel Vetter wrote:
>> On Wed, Nov 29, 2017 at 10:08:55PM -0500, Sean Paul wrote:
>> > Here's the RFC for my i915 HDCP patchset. The UABI is based on what we've 
>> > been
>> > using in Chrome for the past 3 years. I posted the property to the list 
>> > back
>> > then, but never had a mainline driver to implement it. I do now :-)
>> >
>> > Things are mostly in place, danvet gave me some feedback that I will
>> > incorporate in v1. However, in the interest of gaining more early 
>> > feedback, I'm
>> > posting this now.
>> >
>> > TODO:
>> > - Add kerneldoc for property
>>
>> The big thing I'd like to see here is clear description of the flows
>> between kernel and userspace (since there's no helpers on the kernel side
>> to standardize this).
>>
>> One thing we discussed in that context is the addition of an uevent (like
>> we do for anything else that changes with connectors, link_status is one
>> example). But since the hdcp spec explicitly demands that the video player
>> must poll the status an event is moot and won't be used. And I'm no fan of
>> speculative uapi :-)
>
> Speaking of uapi... Do we have an open source userspace and igts for
> this somewhere?
>

The userspace is implemented in Chrome for Chrome OS:
https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_display.cc

I posted an igt test on the list earlier today.

Sean

> I'm also concerned about debugging this. It will be a real PITA
> to do if we can't even test it easily.
>
>>
>> > - Fix '//' comments
>> > - Change to MIT license
>> > - Rebase on Ville's gmbus fixes (thanks Ville)
>> > - Improve documentation on drm_intel_hdcp_shim
>> > - Fix async commit locking (ie: don't use connection_mutex)
>> > - Don't change connector->state in enable, defer to worker
>>
>> Same holds for the disable callback, you can't touch state in there.
>>
>> With the link_status prop (which is somewhat similar) we only reset it in
>> atomic_check (where we hold the state locks) and in the async worker (same
>> applies).
>> -Daniel
>>
>> > - Add igt coverage for the feature.
>> >
>> > Thanks!
>> >
>> > Sean
>> >
>> >
>> > Sean Paul (6):
>> >   drm: Add Content Protection property
>> >   drm: Add some HDCP related #defines
>> >   drm/i915: Add HDCP framework + base implementation
>> >   drm/i915: Add function to output Aksv over GMBUS
>> >   drm/i915: Implement HDCP for HDMI
>> >   drm/i915: Implement HDCP for DisplayPort
>> >
>> >  drivers/gpu/drm/drm_atomic.c|   8 +
>> >  drivers/gpu/drm/drm_connector.c |  43 +++
>> >  drivers/gpu/drm/drm_sysfs.c |  29 ++
>> >  drivers/gpu/drm/i915/Makefile   |   1 +
>> >  drivers/gpu/drm/i915/i915_drv.h |   1 +
>> >  drivers/gpu/drm/i915/i915_reg.h |  85 +
>> >  drivers/gpu/drm/i915/intel_atomic.c |  26 +-
>> >  drivers/gpu/drm/i915/intel_ddi.c|  64 
>> >  drivers/gpu/drm/i915/intel_dp.c | 243 +-
>> >  drivers/gpu/drm/i915/intel_drv.h|  53 +++
>> >  drivers/gpu/drm/i915/intel_hdcp.c   | 636 
>> > 
>> >  drivers/gpu/drm/i915/intel_hdmi.c   | 253 ++
>> >  drivers/gpu/drm/i915/intel_i2c.c|  54 ++-
>> >  include/drm/drm_connector.h |  16 +
>> >  include/drm/drm_dp_helper.h |  17 +
>> >  include/drm/drm_hdcp.h  |  44 +++
>> >  include/uapi/drm/drm_mode.h |   4 +
>> >  17 files changed, 1560 insertions(+), 17 deletions(-)
>> >  create mode 100644 drivers/gpu/drm/i915/intel_hdcp.c
>> >  create mode 100644 include/drm/drm_hdcp.h
>> >
>> > --
>> > 2.15.0.531.g2ccb3012c9-goog
>> >
>> > ___
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC PATCH 0/6] drm/i915: Implement HDCP

2017-12-05 Thread Ville Syrjälä 
On Thu, Nov 30, 2017 at 08:50:38AM +0100, Daniel Vetter wrote:
> On Wed, Nov 29, 2017 at 10:08:55PM -0500, Sean Paul wrote:
> > Here's the RFC for my i915 HDCP patchset. The UABI is based on what we've 
> > been
> > using in Chrome for the past 3 years. I posted the property to the list back
> > then, but never had a mainline driver to implement it. I do now :-)
> > 
> > Things are mostly in place, danvet gave me some feedback that I will
> > incorporate in v1. However, in the interest of gaining more early feedback, 
> > I'm
> > posting this now.
> > 
> > TODO:
> > - Add kerneldoc for property
> 
> The big thing I'd like to see here is clear description of the flows
> between kernel and userspace (since there's no helpers on the kernel side
> to standardize this).
> 
> One thing we discussed in that context is the addition of an uevent (like
> we do for anything else that changes with connectors, link_status is one
> example). But since the hdcp spec explicitly demands that the video player
> must poll the status an event is moot and won't be used. And I'm no fan of
> speculative uapi :-)

Speaking of uapi... Do we have an open source userspace and igts for
this somewhere?

I'm also concerned about debugging this. It will be a real PITA
to do if we can't even test it easily.

> 
> > - Fix '//' comments
> > - Change to MIT license
> > - Rebase on Ville's gmbus fixes (thanks Ville)
> > - Improve documentation on drm_intel_hdcp_shim
> > - Fix async commit locking (ie: don't use connection_mutex)
> > - Don't change connector->state in enable, defer to worker
> 
> Same holds for the disable callback, you can't touch state in there.
> 
> With the link_status prop (which is somewhat similar) we only reset it in
> atomic_check (where we hold the state locks) and in the async worker (same
> applies).
> -Daniel
> 
> > - Add igt coverage for the feature.
> > 
> > Thanks!
> > 
> > Sean
> > 
> > 
> > Sean Paul (6):
> >   drm: Add Content Protection property
> >   drm: Add some HDCP related #defines
> >   drm/i915: Add HDCP framework + base implementation
> >   drm/i915: Add function to output Aksv over GMBUS
> >   drm/i915: Implement HDCP for HDMI
> >   drm/i915: Implement HDCP for DisplayPort
> > 
> >  drivers/gpu/drm/drm_atomic.c|   8 +
> >  drivers/gpu/drm/drm_connector.c |  43 +++
> >  drivers/gpu/drm/drm_sysfs.c |  29 ++
> >  drivers/gpu/drm/i915/Makefile   |   1 +
> >  drivers/gpu/drm/i915/i915_drv.h |   1 +
> >  drivers/gpu/drm/i915/i915_reg.h |  85 +
> >  drivers/gpu/drm/i915/intel_atomic.c |  26 +-
> >  drivers/gpu/drm/i915/intel_ddi.c|  64 
> >  drivers/gpu/drm/i915/intel_dp.c | 243 +-
> >  drivers/gpu/drm/i915/intel_drv.h|  53 +++
> >  drivers/gpu/drm/i915/intel_hdcp.c   | 636 
> > 
> >  drivers/gpu/drm/i915/intel_hdmi.c   | 253 ++
> >  drivers/gpu/drm/i915/intel_i2c.c|  54 ++-
> >  include/drm/drm_connector.h |  16 +
> >  include/drm/drm_dp_helper.h |  17 +
> >  include/drm/drm_hdcp.h  |  44 +++
> >  include/uapi/drm/drm_mode.h |   4 +
> >  17 files changed, 1560 insertions(+), 17 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/intel_hdcp.c
> >  create mode 100644 include/drm/drm_hdcp.h
> > 
> > -- 
> > 2.15.0.531.g2ccb3012c9-goog
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC PATCH 0/6] drm/i915: Implement HDCP

2017-11-29 Thread Daniel Vetter 
On Wed, Nov 29, 2017 at 10:08:55PM -0500, Sean Paul wrote:
> Here's the RFC for my i915 HDCP patchset. The UABI is based on what we've been
> using in Chrome for the past 3 years. I posted the property to the list back
> then, but never had a mainline driver to implement it. I do now :-)
> 
> Things are mostly in place, danvet gave me some feedback that I will
> incorporate in v1. However, in the interest of gaining more early feedback, 
> I'm
> posting this now.
> 
> TODO:
> - Add kerneldoc for property

The big thing I'd like to see here is clear description of the flows
between kernel and userspace (since there's no helpers on the kernel side
to standardize this).

One thing we discussed in that context is the addition of an uevent (like
we do for anything else that changes with connectors, link_status is one
example). But since the hdcp spec explicitly demands that the video player
must poll the status an event is moot and won't be used. And I'm no fan of
speculative uapi :-)

> - Fix '//' comments
> - Change to MIT license
> - Rebase on Ville's gmbus fixes (thanks Ville)
> - Improve documentation on drm_intel_hdcp_shim
> - Fix async commit locking (ie: don't use connection_mutex)
> - Don't change connector->state in enable, defer to worker

Same holds for the disable callback, you can't touch state in there.

With the link_status prop (which is somewhat similar) we only reset it in
atomic_check (where we hold the state locks) and in the async worker (same
applies).
-Daniel

> - Add igt coverage for the feature.
> 
> Thanks!
> 
> Sean
> 
> 
> Sean Paul (6):
>   drm: Add Content Protection property
>   drm: Add some HDCP related #defines
>   drm/i915: Add HDCP framework + base implementation
>   drm/i915: Add function to output Aksv over GMBUS
>   drm/i915: Implement HDCP for HDMI
>   drm/i915: Implement HDCP for DisplayPort
> 
>  drivers/gpu/drm/drm_atomic.c|   8 +
>  drivers/gpu/drm/drm_connector.c |  43 +++
>  drivers/gpu/drm/drm_sysfs.c |  29 ++
>  drivers/gpu/drm/i915/Makefile   |   1 +
>  drivers/gpu/drm/i915/i915_drv.h |   1 +
>  drivers/gpu/drm/i915/i915_reg.h |  85 +
>  drivers/gpu/drm/i915/intel_atomic.c |  26 +-
>  drivers/gpu/drm/i915/intel_ddi.c|  64 
>  drivers/gpu/drm/i915/intel_dp.c | 243 +-
>  drivers/gpu/drm/i915/intel_drv.h|  53 +++
>  drivers/gpu/drm/i915/intel_hdcp.c   | 636 
> 
>  drivers/gpu/drm/i915/intel_hdmi.c   | 253 ++
>  drivers/gpu/drm/i915/intel_i2c.c|  54 ++-
>  include/drm/drm_connector.h |  16 +
>  include/drm/drm_dp_helper.h |  17 +
>  include/drm/drm_hdcp.h  |  44 +++
>  include/uapi/drm/drm_mode.h |   4 +
>  17 files changed, 1560 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_hdcp.c
>  create mode 100644 include/drm/drm_hdcp.h
> 
> -- 
> 2.15.0.531.g2ccb3012c9-goog
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [RFC PATCH 0/6] drm/i915: Implement HDCP

2017-11-29 Thread Sean Paul 
Here's the RFC for my i915 HDCP patchset. The UABI is based on what we've been
using in Chrome for the past 3 years. I posted the property to the list back
then, but never had a mainline driver to implement it. I do now :-)

Things are mostly in place, danvet gave me some feedback that I will
incorporate in v1. However, in the interest of gaining more early feedback, I'm
posting this now.

TODO:
- Add kerneldoc for property
- Fix '//' comments
- Change to MIT license
- Rebase on Ville's gmbus fixes (thanks Ville)
- Improve documentation on drm_intel_hdcp_shim
- Fix async commit locking (ie: don't use connection_mutex)
- Don't change connector->state in enable, defer to worker
- Add igt coverage for the feature.

Thanks!

Sean


Sean Paul (6):
  drm: Add Content Protection property
  drm: Add some HDCP related #defines
  drm/i915: Add HDCP framework + base implementation
  drm/i915: Add function to output Aksv over GMBUS
  drm/i915: Implement HDCP for HDMI
  drm/i915: Implement HDCP for DisplayPort

 drivers/gpu/drm/drm_atomic.c|   8 +
 drivers/gpu/drm/drm_connector.c |  43 +++
 drivers/gpu/drm/drm_sysfs.c |  29 ++
 drivers/gpu/drm/i915/Makefile   |   1 +
 drivers/gpu/drm/i915/i915_drv.h |   1 +
 drivers/gpu/drm/i915/i915_reg.h |  85 +
 drivers/gpu/drm/i915/intel_atomic.c |  26 +-
 drivers/gpu/drm/i915/intel_ddi.c|  64 
 drivers/gpu/drm/i915/intel_dp.c | 243 +-
 drivers/gpu/drm/i915/intel_drv.h|  53 +++
 drivers/gpu/drm/i915/intel_hdcp.c   | 636 
 drivers/gpu/drm/i915/intel_hdmi.c   | 253 ++
 drivers/gpu/drm/i915/intel_i2c.c|  54 ++-
 include/drm/drm_connector.h |  16 +
 include/drm/drm_dp_helper.h |  17 +
 include/drm/drm_hdcp.h  |  44 +++
 include/uapi/drm/drm_mode.h |   4 +
 17 files changed, 1560 insertions(+), 17 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_hdcp.c
 create mode 100644 include/drm/drm_hdcp.h

-- 
2.15.0.531.g2ccb3012c9-goog

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx