Re: [Freedreno] [PATCH v5 03/10] drm/hdcp: Update property value on content type and user changes
On Thu, Apr 14, 2022 at 03:58:02PM +, Sean Paul wrote:
> On Tue, Apr 12, 2022 at 09:25:59AM -0400, Rodrigo Vivi wrote:
> > On Mon, Apr 11, 2022 at 08:47:32PM +, Sean Paul wrote:
> > > From: Sean Paul
> > >
> > > This patch updates the connector's property value in 2 cases which were
> > > previously missed:
> > >
> > > 1- Content type changes. The value should revert back to DESIRED from
> > >ENABLED in case the driver must re-authenticate the link due to the
> > >new content type.
> > >
> > > 2- Userspace sets value to DESIRED while ENABLED. In this case, the
> > >value should be reset immediately to ENABLED since the link is
> > >actively being encrypted.
> > >
> > > To accommodate these changes, I've split up the conditionals to make
> > > things a bit more clear (as much as one can with this mess of state).
> > >
> > > Acked-by: Jani Nikula
> > > Reviewed-by: Abhinav Kumar
> > > Signed-off-by: Sean Paul
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-s...@poorly.run
> > > #v1
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-s...@poorly.run
> > > #v2
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-4-s...@poorly.run
> > > #v3
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-4-s...@poorly.run
> > > #v4
> > >
> > > Changes in v2:
> > > -None
> > > Changes in v3:
> > > -Fixed indentation issue identified by 0-day
> > > Changes in v4:
> > > -None
> > > Changes in v5:
> > > -None
> > > ---
> > > drivers/gpu/drm/drm_hdcp.c | 26 +-
> > > 1 file changed, 17 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> > > index dd8fa91c51d6..8c851d40cd45 100644
> > > --- a/drivers/gpu/drm/drm_hdcp.c
> > > +++ b/drivers/gpu/drm/drm_hdcp.c
> > > @@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector
> > > *connector,
> > > return true;
> > >
> > > /*
> > > - * Nothing to do if content type is unchanged and one of:
> > > - * - state didn't change
> > > + * Content type changes require an HDCP disable/enable cycle.
> > > + */
> > > + if (new_conn_state->hdcp_content_type !=
> > > old_conn_state->hdcp_content_type) {
> >
> > shouldn't we add some && ( old_hdcp ==
> > DRM_MODE_CONTENT_PROTECTION_ENABLED)) {
> > here?
>
> Thanks for your reviews Rodrigo.
>
> I don't think so since the content type is changing the current state of old
> content protection is immaterial (ie: if we need to enable HDCP 2.x, the state
> of HDCP 1.x doesn't really matter), we need to re-evaluate whether the current
> level of HDCP is sufficient.
>
> Hopefully that makes sense, but I could be missing something :-)
nope, I think I was just missing that the important is the content type change
as you pointed out in the item number 1 in the commit msg.
Reviewed-by: Rodrigo Vivi
>
> Sean
>
> >
> > > + new_conn_state->content_protection =
> > > + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> > > + return true;
> > > + }
> > > +
> > > + /*
> > > + * Ignore meaningless state changes:
> > >* - HDCP was activated since the last commit
> > > - * - attempting to set to desired while already enabled
> > > + * - Attempting to set to desired while already enabled
> > >*/
> > > - if (old_hdcp == new_hdcp ||
> > > - (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> > > + if ((old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> > >new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
> > > (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> > >new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> > > - if (old_conn_state->hdcp_content_type ==
> > > - new_conn_state->hdcp_content_type)
> > > - return false;
> > > + new_conn_state->content_protection =
> > > + DRM_MODE_CONTENT_PROTECTION_ENABLED;
> > > + return false;
> > > }
> > >
> > > - return true;
> > > + /* Finally, if state changes, we need action */
> > > + return old_hdcp != new_hdcp;
> > > }
> > > EXPORT_SYMBOL(drm_hdcp_atomic_check);
> > > --
> > > Sean Paul, Software Engineer, Google / Chromium OS
> > >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
Re: [Freedreno] [PATCH v5 03/10] drm/hdcp: Update property value on content type and user changes
On Tue, Apr 12, 2022 at 09:25:59AM -0400, Rodrigo Vivi wrote:
> On Mon, Apr 11, 2022 at 08:47:32PM +, Sean Paul wrote:
> > From: Sean Paul
> >
> > This patch updates the connector's property value in 2 cases which were
> > previously missed:
> >
> > 1- Content type changes. The value should revert back to DESIRED from
> >ENABLED in case the driver must re-authenticate the link due to the
> >new content type.
> >
> > 2- Userspace sets value to DESIRED while ENABLED. In this case, the
> >value should be reset immediately to ENABLED since the link is
> >actively being encrypted.
> >
> > To accommodate these changes, I've split up the conditionals to make
> > things a bit more clear (as much as one can with this mess of state).
> >
> > Acked-by: Jani Nikula
> > Reviewed-by: Abhinav Kumar
> > Signed-off-by: Sean Paul
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-s...@poorly.run
> > #v1
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-s...@poorly.run
> > #v2
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-4-s...@poorly.run
> > #v3
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-4-s...@poorly.run
> > #v4
> >
> > Changes in v2:
> > -None
> > Changes in v3:
> > -Fixed indentation issue identified by 0-day
> > Changes in v4:
> > -None
> > Changes in v5:
> > -None
> > ---
> > drivers/gpu/drm/drm_hdcp.c | 26 +-
> > 1 file changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> > index dd8fa91c51d6..8c851d40cd45 100644
> > --- a/drivers/gpu/drm/drm_hdcp.c
> > +++ b/drivers/gpu/drm/drm_hdcp.c
> > @@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector
> > *connector,
> > return true;
> >
> > /*
> > -* Nothing to do if content type is unchanged and one of:
> > -* - state didn't change
> > +* Content type changes require an HDCP disable/enable cycle.
> > +*/
> > + if (new_conn_state->hdcp_content_type !=
> > old_conn_state->hdcp_content_type) {
>
> shouldn't we add some && ( old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED)) {
> here?
Thanks for your reviews Rodrigo.
I don't think so since the content type is changing the current state of old
content protection is immaterial (ie: if we need to enable HDCP 2.x, the state
of HDCP 1.x doesn't really matter), we need to re-evaluate whether the current
level of HDCP is sufficient.
Hopefully that makes sense, but I could be missing something :-)
Sean
>
> > + new_conn_state->content_protection =
> > + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> > + return true;
> > + }
> > +
> > + /*
> > +* Ignore meaningless state changes:
> > * - HDCP was activated since the last commit
> > -* - attempting to set to desired while already enabled
> > +* - Attempting to set to desired while already enabled
> > */
> > - if (old_hdcp == new_hdcp ||
> > - (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> > + if ((old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> > new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
> > (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> > new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> > - if (old_conn_state->hdcp_content_type ==
> > - new_conn_state->hdcp_content_type)
> > - return false;
> > + new_conn_state->content_protection =
> > + DRM_MODE_CONTENT_PROTECTION_ENABLED;
> > + return false;
> > }
> >
> > - return true;
> > + /* Finally, if state changes, we need action */
> > + return old_hdcp != new_hdcp;
> > }
> > EXPORT_SYMBOL(drm_hdcp_atomic_check);
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
> >
--
Sean Paul, Software Engineer, Google / Chromium OS
Re: [Freedreno] [PATCH v5 03/10] drm/hdcp: Update property value on content type and user changes
On Mon, Apr 11, 2022 at 08:47:32PM +, Sean Paul wrote:
> From: Sean Paul
>
> This patch updates the connector's property value in 2 cases which were
> previously missed:
>
> 1- Content type changes. The value should revert back to DESIRED from
>ENABLED in case the driver must re-authenticate the link due to the
>new content type.
>
> 2- Userspace sets value to DESIRED while ENABLED. In this case, the
>value should be reset immediately to ENABLED since the link is
>actively being encrypted.
>
> To accommodate these changes, I've split up the conditionals to make
> things a bit more clear (as much as one can with this mess of state).
>
> Acked-by: Jani Nikula
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Sean Paul
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-s...@poorly.run
> #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-s...@poorly.run
> #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-4-s...@poorly.run
> #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-4-s...@poorly.run
> #v4
>
> Changes in v2:
> -None
> Changes in v3:
> -Fixed indentation issue identified by 0-day
> Changes in v4:
> -None
> Changes in v5:
> -None
> ---
> drivers/gpu/drm/drm_hdcp.c | 26 +-
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index dd8fa91c51d6..8c851d40cd45 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector
> *connector,
> return true;
>
> /*
> - * Nothing to do if content type is unchanged and one of:
> - * - state didn't change
> + * Content type changes require an HDCP disable/enable cycle.
> + */
> + if (new_conn_state->hdcp_content_type !=
> old_conn_state->hdcp_content_type) {
shouldn't we add some && ( old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED)) {
here?
> + new_conn_state->content_protection =
> + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + return true;
> + }
> +
> + /*
> + * Ignore meaningless state changes:
>* - HDCP was activated since the last commit
> - * - attempting to set to desired while already enabled
> + * - Attempting to set to desired while already enabled
>*/
> - if (old_hdcp == new_hdcp ||
> - (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> + if ((old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
>new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
> (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
>new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> - if (old_conn_state->hdcp_content_type ==
> - new_conn_state->hdcp_content_type)
> - return false;
> + new_conn_state->content_protection =
> + DRM_MODE_CONTENT_PROTECTION_ENABLED;
> + return false;
> }
>
> - return true;
> + /* Finally, if state changes, we need action */
> + return old_hdcp != new_hdcp;
> }
> EXPORT_SYMBOL(drm_hdcp_atomic_check);
> --
> Sean Paul, Software Engineer, Google / Chromium OS
>
[Freedreno] [PATCH v5 03/10] drm/hdcp: Update property value on content type and user changes
From: Sean Paul
This patch updates the connector's property value in 2 cases which were
previously missed:
1- Content type changes. The value should revert back to DESIRED from
ENABLED in case the driver must re-authenticate the link due to the
new content type.
2- Userspace sets value to DESIRED while ENABLED. In this case, the
value should be reset immediately to ENABLED since the link is
actively being encrypted.
To accommodate these changes, I've split up the conditionals to make
things a bit more clear (as much as one can with this mess of state).
Acked-by: Jani Nikula
Reviewed-by: Abhinav Kumar
Signed-off-by: Sean Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-s...@poorly.run
#v1
Link:
https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-s...@poorly.run
#v2
Link:
https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-4-s...@poorly.run
#v3
Link:
https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-4-s...@poorly.run
#v4
Changes in v2:
-None
Changes in v3:
-Fixed indentation issue identified by 0-day
Changes in v4:
-None
Changes in v5:
-None
---
drivers/gpu/drm/drm_hdcp.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
index dd8fa91c51d6..8c851d40cd45 100644
--- a/drivers/gpu/drm/drm_hdcp.c
+++ b/drivers/gpu/drm/drm_hdcp.c
@@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector
*connector,
return true;
/*
-* Nothing to do if content type is unchanged and one of:
-* - state didn't change
+* Content type changes require an HDCP disable/enable cycle.
+*/
+ if (new_conn_state->hdcp_content_type !=
old_conn_state->hdcp_content_type) {
+ new_conn_state->content_protection =
+ DRM_MODE_CONTENT_PROTECTION_DESIRED;
+ return true;
+ }
+
+ /*
+* Ignore meaningless state changes:
* - HDCP was activated since the last commit
-* - attempting to set to desired while already enabled
+* - Attempting to set to desired while already enabled
*/
- if (old_hdcp == new_hdcp ||
- (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
+ if ((old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
(old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
- if (old_conn_state->hdcp_content_type ==
- new_conn_state->hdcp_content_type)
- return false;
+ new_conn_state->content_protection =
+ DRM_MODE_CONTENT_PROTECTION_ENABLED;
+ return false;
}
- return true;
+ /* Finally, if state changes, we need action */
+ return old_hdcp != new_hdcp;
}
EXPORT_SYMBOL(drm_hdcp_atomic_check);
--
Sean Paul, Software Engineer, Google / Chromium OS
