| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<BY5PR04MB6739C5804F0E9764EFD3A3EBC73B2@BY5PR04MB6739.namprd04.prod.outlook.com>
Date: Mon, 16 Dec 2024 08:33:23 +0000
From: Xin Ji <xji@...logixsemi.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong
<neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, Laurent Pinchart
<Laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>, Jernej
Skrabec <jernej.skrabec@...il.com>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Bernie Liang <bliang@...logixsemi.com>,
Qilin Wen <qwen@...logixsemi.com>, "treapking@...gle.com"
<treapking@...gle.com>, "dri-devel@...ts.freedesktop.org"
<dri-devel@...ts.freedesktop.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3] drm/bridge:anx7625: Update HDCP status at
atomic_enable()
> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> Sent: Friday, December 13, 2024 9:17 PM
> To: Xin Ji <xji@...logixsemi.com>
> Cc: Andrzej Hajda <andrzej.hajda@...el.com>; Neil Armstrong
> <neil.armstrong@...aro.org>; Robert Foss <rfoss@...nel.org>; Laurent Pinchart
> <Laurent.pinchart@...asonboard.com>; Jonas Karlman <jonas@...boo.se>;
> Jernej Skrabec <jernej.skrabec@...il.com>; Maarten Lankhorst
> <maarten.lankhorst@...ux.intel.com>; Maxime Ripard <mripard@...nel.org>;
> Thomas Zimmermann <tzimmermann@...e.de>; David Airlie
> <airlied@...il.com>; Simona Vetter <simona@...ll.ch>; Bernie Liang
> <bliang@...logixsemi.com>; Qilin Wen <qwen@...logixsemi.com>;
> treapking@...gle.com; dri-devel@...ts.freedesktop.org; linux-
> kernel@...r.kernel.org
> Subject: Re: [PATCH v3] drm/bridge:anx7625: Update HDCP status at
> atomic_enable()
>
> CAUTION: This email originated from outside of the organization. Please do not
> click links or open attachments unless you recognize the sender, and know the
> content is safe.
>
>
> On Fri, 13 Dec 2024 at 13:00, Xin Ji <xji@...logixsemi.com> wrote:
> >
> > Hi Dmitry, sorry, I didn't clear describe the reason.
>
> Please. Do not top-post. Please paste your answer under the question, not
> somewhere at the top of the email. This allows us to have a more constructive
> dialog. Additional bonus if you can fix your email client to insert sensible quoting
> information instead of dumping the headers of the original email.
Hi Dmitry, OK, sorry about it. Currently, we have problem to fetch email from
Microsoft on Ubuntu. I'll try to fix it later.
>
> >
> > Anx7625 implement DSI to DP convert behind USB Type-C port, when user
> > plug into USB Type-C Dongle with DP monitor, the user space will
> > enable HDCP feature, then kernel do HDCP and output display and set
> > HDCP content to ENABLE, but the issue happened if user manually change the
> monitor's resolution later.
> >
> > Each time user change the resolution, kernel will call bridge
> > interface .atomic_disable() and .atomic_enable(), the old driver will
> > keep HDCP state to ENABLE, this is a BUG, when user change the
> > resolution, kernel must change HDCP content too (mustn't keep to
> > ENABLE),
>
> Why? Could you please point me to the corresponding documentation or a code
> path in the other driver? Preferably i915, AMD or Nouveau.
As https://elixir.bootlin.com/linux/v6.12.5/source/drivers/gpu/drm/drm_connector.c#L1423:
- ENABLED -> DESIRED (termination of authentication)
As there is no other interface to tell anx7625 bridge driver, so the I think best place to handle
ENABLE -> DESIRED in .atomic_disable().
>
> > as DRM doc said, kernel cannot change from ENABLE to UNDESIRE, so next
> > patch, I'll change it to DESIRE in .atomic_disable().
> >
> > Thanks!
> > Xin
> >
> > > -----Original Message-----
> > > From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> > > Sent: Friday, December 13, 2024 6:47 PM
> > > To: Xin Ji <xji@...logixsemi.com>
> > > Cc: Andrzej Hajda <andrzej.hajda@...el.com>; Neil Armstrong
> > > <neil.armstrong@...aro.org>; Robert Foss <rfoss@...nel.org>; Laurent
> > > Pinchart <Laurent.pinchart@...asonboard.com>; Jonas Karlman
> > > <jonas@...boo.se>; Jernej Skrabec <jernej.skrabec@...il.com>;
> > > Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>; Maxime Ripard
> > > <mripard@...nel.org>; Thomas Zimmermann <tzimmermann@...e.de>;
> David
> > > Airlie <airlied@...il.com>; Simona Vetter <simona@...ll.ch>; Bernie
> > > Liang <bliang@...logixsemi.com>; Qilin Wen <qwen@...logixsemi.com>;
> > > treapking@...gle.com; dri-devel@...ts.freedesktop.org; linux-
> > > kernel@...r.kernel.org
> > > Subject: Re: [PATCH v3] drm/bridge:anx7625: Update HDCP status at
> > > atomic_enable()
> > >
> > > CAUTION: This email originated from outside of the organization.
> > > Please do not click links or open attachments unless you recognize
> > > the sender, and know the content is safe.
> > >
> > >
> > > On Fri, Dec 13, 2024 at 10:06:36AM +0000, Xin Ji wrote:
> > > > Hi Dmitry, thanks for the review, I made some changes which change
> > > > ENABLE to DESIRE in .atomic_disable(), I'll upstream it after testing. Thanks!
> > >
> > > - Please don't top-post.
> > >
> > > - You still didn't explain, why do you want to do this change of HDCP
> > > status. Could you please provide an explanation before sending the
> > > next iteration?
> > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> > > > > Sent: Thursday, December 12, 2024 5:18 PM
> > > > > To: Xin Ji <xji@...logixsemi.com>
> > > > > Cc: Andrzej Hajda <andrzej.hajda@...el.com>; Neil Armstrong
> > > > > <neil.armstrong@...aro.org>; Robert Foss <rfoss@...nel.org>;
> > > > > Laurent Pinchart <Laurent.pinchart@...asonboard.com>; Jonas
> > > > > Karlman <jonas@...boo.se>; Jernej Skrabec
> > > > > <jernej.skrabec@...il.com>; Maarten Lankhorst
> > > > > <maarten.lankhorst@...ux.intel.com>; Maxime Ripard
> > > > > <mripard@...nel.org>; Thomas Zimmermann <tzimmermann@...e.de>;
> > > David
> > > > > Airlie <airlied@...il.com>; Simona Vetter <simona@...ll.ch>;
> > > > > Bernie Liang <bliang@...logixsemi.com>; Qilin Wen
> > > > > <qwen@...logixsemi.com>; treapking@...gle.com;
> > > > > dri-devel@...ts.freedesktop.org; linux- kernel@...r.kernel.org
> > > > > Subject: Re: [PATCH v3] drm/bridge:anx7625: Update HDCP status
> > > > > at
> > > > > atomic_enable()
> > > > >
> > > > > CAUTION: This email originated from outside of the organization.
> > > > > Please do not click links or open attachments unless you
> > > > > recognize the sender, and know the content is safe.
> > > > >
> > > > >
> > > > > On Thu, Dec 12, 2024 at 01:51:10PM +0800, Xin Ji wrote:
> > > > > > When user enabled HDCP feature, userspace will set HDCP
> > > > > > content to DRM_MODE_CONTENT_PROTECTION_DESIRED. Next,
> anx7625
> > > > > > will
> > > update
> > > > > HDCP
> > > > > > content to DRM_MODE_CONTENT_PROTECTION_ENABLED if down
> > > stream
> > > > > support
> > > > > > HDCP feature.
> > > > > >
> > > > > > However once HDCP content turn to
> > > > > DRM_MODE_CONTENT_PROTECTION_ENABLED
> > > > > > userspace will not update the HDCP content to
> > > > > > DRM_MODE_CONTENT_PROTECTION_UNDESIRED until monitor
> > > disconnect.
> > > > >
> > > > > It seems you've ingored a part of the previous review comment.
> > > > > It's the userspace who triggers the ENABLED -> UNDESIRED
> > > > > transition, not the kernel side. The change to move HDCP
> > > > > handling to atomic_enable() looks fine, the change to disable
> > > > > HDCP is not (unless I misunderstand
> > > something).
> > > > >
> > > > > >
> > > > > > So, anx7625 driver move hdcp content value checking from
> > > > > > bridge interface .atomic_check() to .atomic_enable(), then
> > > > > > update hdcp content according from currently HDCP status. And
> > > > > > also disabled HDCP in bridge interface .atomic_disable().
> > > > > >
> > > > > > Signed-off-by: Xin Ji <xji@...logixsemi.com>
> > > > > > ---
> > > > > > drivers/gpu/drm/bridge/analogix/anx7625.c | 74
> > > > > > ++++++++++++++---------
> > > > > > 1 file changed, 46 insertions(+), 28 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > > > > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > > > > index a2675b121fe4..f96ce5665e8d 100644
> > > > > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > > > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > > > > @@ -861,6 +861,22 @@ static int anx7625_hdcp_disable(struct
> > > > > > anx7625_data
> > > > > *ctx)
> > > > > > TX_HDCP_CTRL0, ~HARD_AUTH_EN &
> > > > > > 0xFF); }
> > > > > >
> > > > > > +static void anx7625_hdcp_disable_and_update_cp(struct
> > > > > > +anx7625_data
> > > > > > +*ctx) {
> > > > > > + struct device *dev = ctx->dev;
> > > > > > +
> > > > > > + if (!ctx->connector)
> > > > > > + return;
> > > > > > +
> > > > > > + anx7625_hdcp_disable(ctx);
> > > > > > +
> > > > > > + ctx->hdcp_cp = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> > > > > > + drm_hdcp_update_content_protection(ctx->connector,
> > > > > > + ctx->hdcp_cp);
> > > > > > +
> > > > > > + dev_dbg(dev, "update CP to UNDESIRE\n"); }
> > > > > > +
> > > > > > static int anx7625_hdcp_enable(struct anx7625_data *ctx) {
> > > > > > u8 bcap;
> > > > > > @@ -2149,34 +2165,6 @@ static int
> > > > > > anx7625_connector_atomic_check(struct
> > > > > anx7625_data *ctx,
> > > > > > if (cp == ctx->hdcp_cp)
> > > > > > return 0;
> > > > > >
> > > > > > - if (cp == DRM_MODE_CONTENT_PROTECTION_DESIRED) {
> > > > > > - if (ctx->dp_en) {
> > > > > > - dev_dbg(dev, "enable HDCP\n");
> > > > > > - anx7625_hdcp_enable(ctx);
> > > > > > -
> > > > > > - queue_delayed_work(ctx->hdcp_workqueue,
> > > > > > - &ctx->hdcp_work,
> > > > > > - msecs_to_jiffies(2000));
> > > > > > - }
> > > > > > - }
> > > > > > -
> > > > > > - if (cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> > > > > > - if (ctx->hdcp_cp !=
> > > DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> > > > > > - dev_err(dev, "current CP is not ENABLED\n");
> > > > > > - return -EINVAL;
> > > > > > - }
> > > > > > - anx7625_hdcp_disable(ctx);
> > > > > > - ctx->hdcp_cp =
> DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> > > > > > - drm_hdcp_update_content_protection(ctx->connector,
> > > > > > - ctx->hdcp_cp);
> > > > > > - dev_dbg(dev, "update CP to UNDESIRE\n");
> > > > > > - }
> > > > > > -
> > > > > > - if (cp == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> > > > > > - dev_err(dev, "Userspace illegal set to PROTECTION ENABLE\n");
> > > > > > - return -EINVAL;
> > > > > > - }
> > > > > > -
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > @@ -2425,6 +2413,8 @@ static void
> > > > > > anx7625_bridge_atomic_enable(struct
> > > > > drm_bridge *bridge,
> > > > > > struct anx7625_data *ctx = bridge_to_anx7625(bridge);
> > > > > > struct device *dev = ctx->dev;
> > > > > > struct drm_connector *connector;
> > > > > > + struct drm_connector_state *conn_state;
> > > > > > + int cp;
> > > > > >
> > > > > > dev_dbg(dev, "drm atomic enable\n");
> > > > > >
> > > > > > @@ -2439,6 +2429,32 @@ static void
> > > > > > anx7625_bridge_atomic_enable(struct
> > > > > drm_bridge *bridge,
> > > > > > _anx7625_hpd_polling(ctx, 5000 * 100);
> > > > > >
> > > > > > anx7625_dp_start(ctx);
> > > > > > +
> > > > > > + conn_state =
> > > > > > + drm_atomic_get_new_connector_state(state->base.state,
> > > > > > + connector);
> > > > > > +
> > > > > > + if (WARN_ON(!conn_state))
> > > > > > + return;
> > > > > > +
> > > > > > + cp = conn_state->content_protection;
> > > > > > + if (cp == DRM_MODE_CONTENT_PROTECTION_DESIRED) {
> > > > > > + if (ctx->dp_en) {
> > > > > > + dev_dbg(dev, "enable HDCP\n");
> > > > > > + anx7625_hdcp_enable(ctx);
> > > > > > +
> > > > > > + queue_delayed_work(ctx->hdcp_workqueue,
> > > > > > + &ctx->hdcp_work,
> > > > > > + msecs_to_jiffies(2000));
> > > > > > + }
> > > > > > + }
> > > > > > +
> > > > > > + if (cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> > > > > > + if (ctx->hdcp_cp !=
> > > DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> > > > > > + dev_err(dev, "current CP is not ENABLED\n");
> > > > > > + return;
> > > > > > + }
> > > > > > +
> > > > > > + anx7625_hdcp_disable_and_update_cp(ctx);
> > > > > > + }
> > > > > > }
> > > > > >
> > > > > > static void anx7625_bridge_atomic_disable(struct drm_bridge
> > > > > > *bridge, @@ -2449,6 +2465,8 @@ static void
> > > > > > anx7625_bridge_atomic_disable(struct
> > > > > > drm_bridge *bridge,
> > > > > >
> > > > > > dev_dbg(dev, "drm atomic disable\n");
> > > > > >
> > > > > > + anx7625_hdcp_disable_and_update_cp(ctx);
> > > > > > +
> > > > > > ctx->connector = NULL;
> > > > > > anx7625_dp_stop(ctx);
> > > > > >
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > > >
> > > > > --
> > > > > With best wishes
> > > > > Dmitry
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
>
>
> --
> With best wishes
> Dmitry
Powered by blists - more mailing lists