<div dir="ltr"><div>It sounds like the kernel should set the hint based on which queues are used, so that every UMD doesn't have to duplicate the same logic.<br></div><div><br></div><div>Marek</div><div><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 22, 2023 at 10:29 AM Christian König <<a href="mailto:christian.koenig@amd.com" target="_blank">christian.koenig@amd.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
Well that sounds like being able to optionally set it after context
creation is actually the right approach.<br>
<br>
VA-API could set it as soon as we know that this is a video codec
application.<br>
<br>
Vulkan can set it depending on what features are used by the
application.<br>
<br>
But yes, Shashank (or whoever requested that) should come up with
some code for Mesa to actually use it. Otherwise we don't have the
justification to push it into the kernel driver.<br>
<br>
Christian.<br>
<br>
<div>Am 22.03.23 um 15:24 schrieb Marek
Olšák:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div>The hint is static per API (one of graphics, video,
compute, unknown). In the case of Vulkan, which exposes all
queues, the hint is unknown, so Vulkan won't use it. (or make
it based on the queue being used and not the uapi context
state) GL won't use it because the default hint is already 3D.
That makes VAAPI the only user that only sets the hint once,
and maybe it's not worth even adding this uapi just for VAAPI.<br>
</div>
<div><br>
</div>
<div>Marek<br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Mar 22, 2023 at
10:08 AM Christian König <<a href="mailto:christian.koenig@amd.com" target="_blank">christian.koenig@amd.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div> Well completely agree that we shouldn't have unused
API. That's why I said we should remove the getting the
hint from the UAPI.<br>
<br>
But what's wrong with setting it after creating the
context? Don't you know enough about the use case? I need
to understand the background a bit better here.<br>
<br>
Christian.<br>
<br>
<div>Am 22.03.23 um 15:05 schrieb Marek Olšák:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div>The option to change the hint after context
creation and get the hint would be unused uapi, and
AFAIK we are not supposed to add unused uapi. What I
asked is to change it to a uapi that userspace will
actually use.<br>
</div>
<div><br>
</div>
<div>Marek<br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Mar 21, 2023
at 9:54 AM Christian König <<a href="mailto:ckoenig.leichtzumerken@gmail.com" target="_blank">ckoenig.leichtzumerken@gmail.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div> Yes, I would like to avoid having multiple
code paths for context creation.<br>
<br>
Setting it later on should be equally to
specifying it on creation since we only need it
during CS.<br>
<br>
Regards,<br>
Christian.<br>
<br>
<div>Am 21.03.23 um 14:00 schrieb Sharma,
Shashank:<br>
</div>
<blockquote type="cite">
<div>
<p style="margin:0cm"><span style="font-size:10pt;font-family:"Arial",sans-serif;color:blue">[AMD
Official Use Only - General]</span></p>
<p class="MsoNormal"> </p>
<p class="MsoNormal"><span>When we started
this patch series, the workload hint was a
part of the ctx_flag only, </span></p>
<p class="MsoNormal"><span>But we changed that
after the design review, to make it more
like how we are handling PSTATE. </span></p>
<p class="MsoNormal"><span> </span></p>
<p class="MsoNormal"><span>Details: </span></p>
<p class="MsoNormal"><span><a href="https://patchwork.freedesktop.org/patch/496111/" target="_blank">https://patchwork.freedesktop.org/patch/496111/</a>
</span></p>
<p class="MsoNormal"><span> </span></p>
<p class="MsoNormal"><span>Regards</span></p>
<p class="MsoNormal"><span>Shashank </span></p>
<p class="MsoNormal"><span> </span></p>
<div style="border-color:rgb(225,225,225) currentcolor currentcolor;border-style:solid none none;border-width:1pt medium medium;padding:3pt 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US">From:</span></b><span lang="EN-US"> Marek Olšák <a href="mailto:maraeo@gmail.com" target="_blank"><maraeo@gmail.com></a>
<br>
<b>Sent:</b> 21 March 2023 04:05<br>
<b>To:</b> Sharma, Shashank <a href="mailto:Shashank.Sharma@amd.com" target="_blank"><Shashank.Sharma@amd.com></a><br>
<b>Cc:</b> <a href="mailto:amd-gfx@lists.freedesktop.org" target="_blank">amd-gfx@lists.freedesktop.org</a>;
Deucher, Alexander <a href="mailto:Alexander.Deucher@amd.com" target="_blank"><Alexander.Deucher@amd.com></a>;
Somalapuram, Amaranath <a href="mailto:Amaranath.Somalapuram@amd.com" target="_blank"><Amaranath.Somalapuram@amd.com></a>;
Koenig, Christian <a href="mailto:Christian.Koenig@amd.com" target="_blank"><Christian.Koenig@amd.com></a><br>
<b>Subject:</b> Re: [PATCH v3 1/5]
drm/amdgpu: add UAPI for workload hints
to ctx ioctl</span></p>
</div>
<p class="MsoNormal"> </p>
<div>
<div>
<p class="MsoNormal">I think we should do
it differently because this interface
will be mostly unused by open source
userspace in its current form.</p>
</div>
<div>
<p class="MsoNormal"> </p>
</div>
<div>
<p class="MsoNormal">Let's set the
workload hint in
drm_amdgpu_ctx_in::flags, and that will
be immutable for the lifetime of the
context. No other interface is needed.</p>
</div>
<div>
<p class="MsoNormal"> </p>
</div>
<div>
<p class="MsoNormal">Marek</p>
</div>
</div>
<p class="MsoNormal"> </p>
<div>
<div>
<p class="MsoNormal">On Mon, Sep 26, 2022
at 5:41 PM Shashank Sharma <<a href="mailto:shashank.sharma@amd.com" target="_blank">shashank.sharma@amd.com</a>>
wrote:</p>
</div>
<blockquote style="border-color:currentcolor currentcolor currentcolor rgb(204,204,204);border-style:none none none solid;border-width:medium medium medium 1pt;padding:0cm 0cm 0cm 6pt;margin:5pt 0cm 5pt 4.8pt">
<p class="MsoNormal" style="margin-bottom:12pt">Allow the
user to specify a workload hint to the
kernel.<br>
We can use these to tweak the dpm
heuristics to better match<br>
the workload for improved performance.<br>
<br>
V3: Create only set() workload UAPI
(Christian)<br>
<br>
Signed-off-by: Alex Deucher <<a href="mailto:alexander.deucher@amd.com" target="_blank">alexander.deucher@amd.com</a>><br>
Signed-off-by: Shashank Sharma <<a href="mailto:shashank.sharma@amd.com" target="_blank">shashank.sharma@amd.com</a>><br>
---<br>
include/uapi/drm/amdgpu_drm.h | 17
+++++++++++++++++<br>
1 file changed, 17 insertions(+)<br>
<br>
diff --git
a/include/uapi/drm/amdgpu_drm.h
b/include/uapi/drm/amdgpu_drm.h<br>
index c2c9c674a223..23d354242699 100644<br>
--- a/include/uapi/drm/amdgpu_drm.h<br>
+++ b/include/uapi/drm/amdgpu_drm.h<br>
@@ -212,6 +212,7 @@ union
drm_amdgpu_bo_list {<br>
#define AMDGPU_CTX_OP_QUERY_STATE2
4<br>
#define
AMDGPU_CTX_OP_GET_STABLE_PSTATE 5<br>
#define
AMDGPU_CTX_OP_SET_STABLE_PSTATE 6<br>
+#define
AMDGPU_CTX_OP_SET_WORKLOAD_PROFILE 7<br>
<br>
/* GPU reset status */<br>
#define AMDGPU_CTX_NO_RESET
0<br>
@@ -252,6 +253,17 @@ union
drm_amdgpu_bo_list {<br>
#define
AMDGPU_CTX_STABLE_PSTATE_MIN_MCLK 3<br>
#define AMDGPU_CTX_STABLE_PSTATE_PEAK
4<br>
<br>
+/* GPU workload hints, flag bits 8-15
*/<br>
+#define AMDGPU_CTX_WORKLOAD_HINT_SHIFT
8<br>
+#define AMDGPU_CTX_WORKLOAD_HINT_MASK
(0xff <<
AMDGPU_CTX_WORKLOAD_HINT_SHIFT)<br>
+#define AMDGPU_CTX_WORKLOAD_HINT_NONE
(0 <<
AMDGPU_CTX_WORKLOAD_HINT_SHIFT)<br>
+#define AMDGPU_CTX_WORKLOAD_HINT_3D
(1 <<
AMDGPU_CTX_WORKLOAD_HINT_SHIFT)<br>
+#define AMDGPU_CTX_WORKLOAD_HINT_VIDEO
(2 <<
AMDGPU_CTX_WORKLOAD_HINT_SHIFT)<br>
+#define AMDGPU_CTX_WORKLOAD_HINT_VR
(3 <<
AMDGPU_CTX_WORKLOAD_HINT_SHIFT)<br>
+#define
AMDGPU_CTX_WORKLOAD_HINT_COMPUTE (4
<< AMDGPU_CTX_WORKLOAD_HINT_SHIFT)<br>
+#define AMDGPU_CTX_WORKLOAD_HINT_MAX
AMDGPU_CTX_WORKLOAD_HINT_COMPUTE<br>
+#define AMDGPU_CTX_WORKLOAD_INDEX(n)
(n >>
AMDGPU_CTX_WORKLOAD_HINT_SHIFT)<br>
+<br>
struct drm_amdgpu_ctx_in {<br>
/** AMDGPU_CTX_OP_* */<br>
__u32 op;<br>
@@ -281,6 +293,11 @@ union
drm_amdgpu_ctx_out {<br>
__u32 flags;<br>
__u32 _pad;<br>
} pstate;<br>
+<br>
+ struct {<br>
+ __u32 flags;<br>
+ __u32 _pad;<br>
+ } workload;<br>
};<br>
<br>
union drm_amdgpu_ctx {<br>
-- <br>
2.34.1</p>
</blockquote>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote></div></div>