On Thu, Sep 25, 2025 at 3:50 PM Mario Limonciello
<[email protected]> wrote:
>
>
>
> On 9/25/2025 2:46 PM, Alex Deucher wrote:
> > On Thu, Sep 25, 2025 at 3:39 PM Mario Limonciello
> > <[email protected]> wrote:
> >>
> >> [Why]
> >> Not all renoir hardware supports secure display. If the TA is present
> >> but the feature isn't supported it will fail to load or send commands.
> >> This shows ERR messages to the user that make it seems like there is
> >> a problem.
> >>
> >> [How]
> >> Check the resp_status of the context to see if there was an error
> >> before trying to send any secure display commands.
> >>
> >> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/1415
> >> Signed-off-by: Mario Limonciello <[email protected]>
> >
> > I think the tricky part is that we want it to throw an error on a
> > system where it is supported so the user knows something is wrong.
>
> But a system that it's supported would have loaded the TA correctly, right?
Yes, it should.
>
> This is specifically checking if the TA load failed which is being used
> as a proxy to show you shouldn't bother with the other commands.
That makes sense, but I don't think it fixes the bug report. The
driver will still report the error on TA load. I'm not sure how we
can avoid that.
Alex
>
> We still show WARN messages from
> psp_ta_load()
> ->psp_cmd_submit_buf()
>
> I guess arguably this change really should be:
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 693357caa9a8..1c790dfccc9f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -2350,7 +2350,7 @@ static int psp_securedisplay_initialize(struct
> psp_context *psp)
> }
>
> ret = psp_ta_load(psp, &psp->securedisplay_context.context);
> - if (!ret) {
> + if (!ret && !psp->securedisplay_context.context.resp_status) {
> psp->securedisplay_context.context.initialized = true;
> mutex_init(&psp->securedisplay_context.mutex);
> } else
> >
> > Alex
> >
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> >> index 693357caa9a8..70d4bfb13f46 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> >> @@ -2356,6 +2356,9 @@ static int psp_securedisplay_initialize(struct
> >> psp_context *psp)
> >> } else
> >> return ret;
> >>
> >> + if (psp->securedisplay_context.context.resp_status)
> >> + return 0;
> >> +
> >> mutex_lock(&psp->securedisplay_context.mutex);
> >>
> >> psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
> >> --
> >> 2.51.0
> >>
>
