Re: [PATCH 6/6] drm/i915: Implement fdinfo memory stats printing
Hi Tvrtko, > Use the newly added drm_print_memory_stats helper to show memory > utilisation of our objects in drm/driver specific fdinfo output. > > To collect the stats we walk the per memory regions object lists > and accumulate object size into the respective drm_memory_stats > categories. > > v2: > * Only account against the active region. > * Use DMA_RESV_USAGE_BOOKKEEP when testing for active. (Tejas) > > v3: > * Update commit text. (Aravind) > * Update to use memory regions uabi names. > > Signed-off-by: Tvrtko Ursulin > Cc: Aravind Iddamsetty > Cc: Rob Clark > Cc: Andi Shyti > Cc: Tejas Upadhyay > Reviewed-by: Andi Shyti # v1 > Reviewed-by: Aravind Iddamsetty # v2 Reviewed-by: Andi Shyti Andi
Re: [PATCH v6 00/16] introduce more MDP3 components in MT8195
On Sat, 2023-09-23 at 19:36 +0200, Krzysztof Kozlowski wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 22/09/2023 09:21, Moudy Ho wrote: > > Changes since v5: > > - Rebase on v6.6-rc2. > > - Dependent dtsi files: > > > https://patchwork.kernel.org/project/linux-mediatek/list/?series=786511 > > - Depends on: > > Message ID = 20230911074233.31556-5-shawn.s...@mediatek.com > > - Split out common propertis for RDMA. > > - Split each component into independent patches. > > And ignored previously given feedback. That's not the way you should > work with upstream community. It feels like a waste of my time and it > is > not fair that Mediatek is doing it :( > > Best regards, > Krzysztof > Hi Krzysztof, While splitting the bindings, I was so focused that I accidentally missed correcting the properties that needed to be fixed. I sincerely apologize for this oversight. Moving forward, I will handle such matters with greater care. Sincerely, Moudy
Re: [PATCH] drm/bridge: ti-sn65dsi83: Do not generate HFP/HBP/HSA and EOT packet
On Mon, Apr 03, 2023 at 09:02:42PM +0200, Marek Vasut wrote: > Do not generate the HS front and back porch gaps, the HSA gap and > EOT packet, as per "SN65DSI83 datasheet SLLSEC1I - SEPTEMBER 2012 > - REVISED OCTOBER 2020", page 22, these packets are not required. > This makes the TI SN65DSI83 bridge work with Samsung DSIM on i.MX8MN. > > Signed-off-by: Marek Vasut Hello, can you please queue up this to kernel v6.1-stable ? commit ca161b259cc84fe1f4a2ce4c73c3832cf6f713f1 Author: Marek Vasut Commit: Robert Foss drm/bridge: ti-sn65dsi83: Do not generate HFP/HBP/HSA and EOT packet Do not generate the HS front and back porch gaps, the HSA gap and EOT packet, as per "SN65DSI83 datasheet SLLSEC1I - SEPTEMBER 2012 - REVISED OCTOBER 2020", page 22, these packets are not required. This makes the TI SN65DSI83 bridge work with Samsung DSIM on i.MX8MN. Signed-off-by: Marek Vasut Reviewed-by: Laurent Pinchart Signed-off-by: Robert Foss Link: https://patchwork.freedesktop.org/patch/msgid/20230403190242.224490-1-ma...@denx.de It solves a real issue with some displays not working without it. Thanks, Francesco
Re: [PATCH 0/6] accel/ivpu: Fixes for linux-6.6-rc4
On Mon, Sep 25, 2023 at 02:11:31PM +0200, Stanislaw Gruszka wrote: > - dmesg flood fix > - HW power-on and interrupt handling fixes for VPU4 > - FW loading/mapping fix Pushed to drm-misc-fixes. I had to resolve conflict when rebuilding tip, hope everything is ok there. Thanks Stanislaw > Jacek Lawrynowicz (1): > accel/ivpu: Don't flood dmesg with VPU ready message > > Karol Wachowski (4): > accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up > accel/ivpu/40xx: Disable frequency change interrupt > accel/ivpu/40xx: Fix missing VPUIP interrupts > accel/ivpu: Use cached buffers for FW loading > > Stanislaw Gruszka (1): > accel/ivpu: Do not use wait event interruptible > > drivers/accel/ivpu/ivpu_drv.c | 2 +- > drivers/accel/ivpu/ivpu_fw.c | 8 +--- > drivers/accel/ivpu/ivpu_gem.h | 5 + > drivers/accel/ivpu/ivpu_hw_40xx.c | 28 +++ > drivers/accel/ivpu/ivpu_hw_40xx_reg.h | 2 ++ > drivers/accel/ivpu/ivpu_ipc.c | 11 --- > 6 files changed, 37 insertions(+), 19 deletions(-) > > -- > 2.25.1 >
Re: [PATCH] drm/i915: Don't set PIPE_CONTROL_FLUSH_L3 for aux inval
On 26.09.2023 16:24, Nirmoy Das wrote:
PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation
so don't set that.
Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before invalidation")
Cc: Jonathan Cavitt
Cc: Andi Shyti
Cc: # v5.8+
Cc: Andrzej Hajda
Cc: Tvrtko Ursulin
Cc: Matt Roper
Cc: Tejas Upadhyay
Cc: Lucas De Marchi
Cc: Prathap Kumar Valsan
Cc: Tapani Pälli
Cc: Mark Janes
Cc: Rodrigo Vivi
Signed-off-by: Nirmoy Das
Reviewed-by: Andrzej Hajda
Regards
Andrzej
---
drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 0143445dba83..ba4c2422b340 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -271,8 +271,17 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70))
bit_group_0 |= PIPE_CONTROL_CCS_FLUSH;
+ /*
+* L3 fabric flush is needed for AUX CCS invalidation
+* which happens as part of pipe-control so we can
+* ignore PIPE_CONTROL_FLUSH_L3. Also PIPE_CONTROL_FLUSH_L3
+* deals with Protected Memory which is not needed for
+* AUX CCS invalidation and lead to unwanted side effects.
+*/
+ if (mode & EMIT_FLUSH)
+ bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
+
bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH;
- bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
bit_group_1 |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
/* Wa_1409600907:tgl,adl-p */
Re: [PATCH 2/2] drm/amd/display: Fix null pointer dereference in error message
Am 26.09.23 um 15:09 schrieb Harry Wentland:
On 2023-09-26 01:56, Cong Liu wrote:
This patch fixes a null pointer dereference in the error message that is
printed when the Display Core (DC) fails to initialize. The original
message includes the DC version number, which is undefined if the DC is
not initialized.
Fixes: 9788d087caff ("drm/amd/display: improve the message printed when loading
DC")
Signed-off-by: Cong Liu
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 8e98dda1e084..bf52a909f558 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1703,8 +1703,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
DRM_INFO("Display Core v%s initialized on %s\n", DC_VER,
dce_version_to_string(adev->dm.dc->ctx->dce_version));
} else {
- DRM_INFO("Display Core v%s failed to initialize on %s\n",
DC_VER,
-dce_version_to_string(adev->dm.dc->ctx->dce_version));
+ DRM_INFO("Display Core failed to initialize with v%s!\n",
DC_VER);
There is value in printing the version number. Let's not remove it.
Instead you can probably fix it by doing a NULL check on adev->dm.dc->ctx.
But as far as I understand it adev->dm.dc->ctx will always be NULL in
this case.
Regards,
Christian.
Harry
goto error;
}
Re: [PATCH 6/6] drm/i915: Implement fdinfo memory stats printing
On 22-09-2023 19:17, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Use the newly added drm_print_memory_stats helper to show memory
> utilisation of our objects in drm/driver specific fdinfo output.
>
> To collect the stats we walk the per memory regions object lists
> and accumulate object size into the respective drm_memory_stats
> categories.
>
> v2:
> * Only account against the active region.
> * Use DMA_RESV_USAGE_BOOKKEEP when testing for active. (Tejas)
>
> v3:
> * Update commit text. (Aravind)
> * Update to use memory regions uabi names.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Aravind Iddamsetty
> Cc: Rob Clark
> Cc: Andi Shyti
> Cc: Tejas Upadhyay
> Reviewed-by: Andi Shyti # v1
> Reviewed-by: Aravind Iddamsetty # v2
Reviewed-by: Aravind Iddamsetty
Thanks,
Aravind.
> ---
> drivers/gpu/drm/i915/i915_drm_client.c | 64 ++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drm_client.c
> b/drivers/gpu/drm/i915/i915_drm_client.c
> index a61356012df8..7efffdaa508d 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.c
> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
> @@ -45,6 +45,68 @@ void __i915_drm_client_free(struct kref *kref)
> }
>
> #ifdef CONFIG_PROC_FS
> +static void
> +obj_meminfo(struct drm_i915_gem_object *obj,
> + struct drm_memory_stats stats[INTEL_REGION_UNKNOWN])
> +{
> + const enum intel_region_id id = obj->mm.region ?
> + obj->mm.region->id : INTEL_REGION_SMEM;
> + const u64 sz = obj->base.size;
> +
> + if (obj->base.handle_count > 1)
> + stats[id].shared += sz;
> + else
> + stats[id].private += sz;
> +
> + if (i915_gem_object_has_pages(obj)) {
> + stats[id].resident += sz;
> +
> + if (!dma_resv_test_signaled(obj->base.resv,
> + DMA_RESV_USAGE_BOOKKEEP))
> + stats[id].active += sz;
> + else if (i915_gem_object_is_shrinkable(obj) &&
> + obj->mm.madv == I915_MADV_DONTNEED)
> + stats[id].purgeable += sz;
> + }
> +}
> +
> +static void show_meminfo(struct drm_printer *p, struct drm_file *file)
> +{
> + struct drm_memory_stats stats[INTEL_REGION_UNKNOWN] = {};
> + struct drm_i915_file_private *fpriv = file->driver_priv;
> + struct i915_drm_client *client = fpriv->client;
> + struct drm_i915_private *i915 = fpriv->i915;
> + struct drm_i915_gem_object *obj;
> + struct intel_memory_region *mr;
> + struct list_head *pos;
> + unsigned int id;
> +
> + /* Public objects. */
> + spin_lock(&file->table_lock);
> + idr_for_each_entry(&file->object_idr, obj, id)
> + obj_meminfo(obj, stats);
> + spin_unlock(&file->table_lock);
> +
> + /* Internal objects. */
> + rcu_read_lock();
> + list_for_each_rcu(pos, &client->objects_list) {
> + obj = i915_gem_object_get_rcu(list_entry(pos, typeof(*obj),
> + client_link));
> + if (!obj)
> + continue;
> + obj_meminfo(obj, stats);
> + i915_gem_object_put(obj);
> + }
> + rcu_read_unlock();
> +
> + for_each_memory_region(mr, i915, id)
> + drm_print_memory_stats(p,
> +&stats[id],
> +DRM_GEM_OBJECT_RESIDENT |
> +DRM_GEM_OBJECT_PURGEABLE,
> +mr->uabi_name);
> +}
> +
> static const char * const uabi_class_names[] = {
> [I915_ENGINE_CLASS_RENDER] = "render",
> [I915_ENGINE_CLASS_COPY] = "copy",
> @@ -106,6 +168,8 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct
> drm_file *file)
>* **
>*/
>
> + show_meminfo(p, file);
> +
> if (GRAPHICS_VER(i915) < 8)
> return;
>
Re: [PATCH 5/6] drm/i915: Add stable memory region names
On 26-09-2023 21:12, Tvrtko Ursulin wrote:
>
> On 26/09/2023 16:29, Iddamsetty, Aravind wrote:
>> On 22-09-2023 19:16, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin
>>>
>>> At the moment memory region names are a bit too varied and too
>>> inconsistent to be used for ABI purposes, like for upcoming fdinfo
>>> memory stats.
>>>
>>> System memory can be either system or system-ttm. Local memory has the
>>> instance number appended, others do not. Not only incosistent but thi
>>> kind of implementation detail is uninteresting for intended users of
>>> fdinfo memory stats.
>>>
>>> Add a stable name always formed as $type$instance. Could have chosen a
>>> different stable scheme, but I think any consistent and stable scheme
>>> should do just fine.
>>>
>>> Signed-off-by: Tvrtko Ursulin
>>> ---
>>> drivers/gpu/drm/i915/intel_memory_region.c | 19 +++
>>> drivers/gpu/drm/i915/intel_memory_region.h | 1 +
>>> 2 files changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_memory_region.c
>>> b/drivers/gpu/drm/i915/intel_memory_region.c
>>> index 3d1fdea9811d..60a03340bbd4 100644
>>> --- a/drivers/gpu/drm/i915/intel_memory_region.c
>>> +++ b/drivers/gpu/drm/i915/intel_memory_region.c
>>> @@ -216,6 +216,22 @@ static int intel_memory_region_memtest(struct
>>> intel_memory_region *mem,
>>> return err;
>>> }
>>> +static const char *region_type_str(u16 type)
>>> +{
>>> + switch (type) {
>>> + case INTEL_MEMORY_SYSTEM:
>>> + return "system";
>>> + case INTEL_MEMORY_LOCAL:
>>> + return "local";
>>> + case INTEL_MEMORY_STOLEN_LOCAL:
>>> + return "stolen-local";
>>> + case INTEL_MEMORY_STOLEN_SYSTEM:
>>> + return "stolen-system";
>>> + default:
>>> + return "unknown";
>>> + }
>>> +}
>>> +
>>> struct intel_memory_region *
>>> intel_memory_region_create(struct drm_i915_private *i915,
>>> resource_size_t start,
>>> @@ -244,6 +260,9 @@ intel_memory_region_create(struct
>>> drm_i915_private *i915,
>>> mem->type = type;
>>> mem->instance = instance;
>>> + snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
>>> + region_type_str(type), instance);
>>> +
>>> mutex_init(&mem->objects.lock);
>>> INIT_LIST_HEAD(&mem->objects.list);
>>> diff --git a/drivers/gpu/drm/i915/intel_memory_region.h
>>> b/drivers/gpu/drm/i915/intel_memory_region.h
>>> index 2953ed5c3248..9ba36454e51b 100644
>>> --- a/drivers/gpu/drm/i915/intel_memory_region.h
>>> +++ b/drivers/gpu/drm/i915/intel_memory_region.h
>>> @@ -80,6 +80,7 @@ struct intel_memory_region {
>>> u16 instance;
>>> enum intel_region_id id;
>>> char name[16];
>>> + char uabi_name[16];
>>
>> Just a thought instead of creating a new field, can't we derive this
>> with name and instance?
>
> I'd rather not snprintf on every fdinfo read - for every pid and every
> drm fd versus 2-3 strings kept around.
ya agreed makes no sense.
>
> I did briefly wonder if mr->name could be dropped, that is renamed to
> mr->uabi_name, but I guess there is some value to print the internal
> name in some log messages, to leave a trace of what underlying
> implementation is used. Although I am not too sure about the value of
> that either since it is implied from the kernel version.
>
> Then on top the usage in i915_gem_create/repr_name I could replace with
> mr->uabi_name and simplify. If there is any value in printing the name
> there, versus just uabi type:instance integers. Dunno. All I know is
> fdinfo should have stable names and not confuse with implementation
> details so I need something..
Ok. LGTM
Reviewed-by: Aravind Iddamsetty
Thanks,
Aravind.
>
> Regards,
>
> Tvrtko
[Patch v1] drm/i915: Add uAPI to query micro-controller FW version
Due to a bug in GuC firmware, Mesa can't enable by default the usage of
compute engines in DG2 and newer.
A new GuC firmware fixed the issue but until now there was no way
for Mesa to know if KMD was running with the fixed GuC version or not,
so this uAPI is required.
It may be expanded in future to query other firmware versions too.
More information:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23661
Mesa usage: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233
Cc: John Harrison
Cc: Daniele Ceraolo Spurio
Cc: José Roberto de Souza
Signed-off-by: Vivaik Balasubrawmanian
---
drivers/gpu/drm/i915/i915_query.c | 47 +++
include/uapi/drm/i915_drm.h | 32 +
2 files changed, 79 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_query.c
b/drivers/gpu/drm/i915/i915_query.c
index 00871ef99792..7f22a49faae7 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -551,6 +551,52 @@ static int query_hwconfig_blob(struct
drm_i915_private *i915,
return hwconfig->size;
}
+static int
+query_uc_fw_version(struct drm_i915_private *i915, struct
drm_i915_query_item *query)
+{
+ struct drm_i915_query_uc_fw_version __user *query_ptr =
u64_to_user_ptr(query->data_ptr);
+ size_t size = sizeof(struct drm_i915_query_uc_fw_version);
+ struct drm_i915_query_uc_fw_version resp;
+
+ if (query->length == 0) {
+ query->length = size;
+ return 0;
+ } else if (query->length != size) {
+ drm_dbg(&i915->drm,
+ "Invalid uc_fw_version query item size=%u expected=%zu\n",
+ query->length, size);
+ return -EINVAL;
+ }
+
+ if (copy_from_user(&resp, query_ptr, size))
+ return -EFAULT;
+
+ if (resp.pad || resp.pad2 || resp.reserved) {
+ drm_dbg(&i915->drm,
+ "Invalid input fw version query structure parameters
received");
+ return -EINVAL;
+ }
+
+ switch (resp.uc_type) {
+ case I915_QUERY_UC_TYPE_GUC: {
+ struct intel_guc *guc = &i915->gt0.uc.guc;
+
+ resp.major_ver = guc->submission_version.major;
+ resp.minor_ver = guc->submission_version.minor;
+ resp.patch_ver = guc->submission_version.patch;
+ resp.branch_ver = 0;
+ break;
+ }
+ default:
+ return -EINVAL;
+ }
+
+ if (copy_to_user(query_ptr, &resp, size))
+ return -EFAULT;
+
+ return 0;
+}
+
static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
struct drm_i915_query_item *query_item) = {
query_topology_info,
@@ -559,6 +605,7 @@ static int (* const i915_query_funcs[])(struct
drm_i915_private *dev_priv,
query_memregion_info,
query_hwconfig_blob,
query_geometry_subslices,
+ query_uc_fw_version,
};
int i915_query_ioctl(struct drm_device *dev, void *data, struct
drm_file *file)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 7000e5910a1d..9be241fb77d8 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3013,6 +3013,7 @@ struct drm_i915_query_item {
* - %DRM_I915_QUERY_MEMORY_REGIONS (see struct
drm_i915_query_memory_regions)
* - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob uAPI`)
* - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct
drm_i915_query_topology_info)
+ * - %DRM_I915_QUERY_UC_FW_VERSION (see struct
drm_i915_query_uc_fw_version)
*/
__u64 query_id;
#define DRM_I915_QUERY_TOPOLOGY_INFO 1
@@ -3021,6 +3022,7 @@ struct drm_i915_query_item {
#define DRM_I915_QUERY_MEMORY_REGIONS 4
#define DRM_I915_QUERY_HWCONFIG_BLOB 5
#define DRM_I915_QUERY_GEOMETRY_SUBSLICES 6
+#define DRM_I915_QUERY_UC_FW_VERSION 7
/* Must be kept compact -- no holes and well documented */
/**
@@ -3213,6 +3215,36 @@ struct drm_i915_query_topology_info {
__u8 data[];
};
+/**
+* struct drm_i915_query_uc_fw_version - query a micro-controller
firmware version
+*
+* Given a uc_type this will return the major, minor, patch and branch
version
+* of the micro-controller firmware.
+*/
+struct drm_i915_query_uc_fw_version {
+ /** @uc: The micro-controller type to query firmware version */
+#define I915_QUERY_UC_TYPE_GUC 0
+ __u16 uc_type;
+
+ /** @pad: MBZ */
+ __u16 pad;
+
+ /* @major_ver: major uc fw version */
+ __u32 major_ver;
+ /* @minor_ver: minor uc fw version */
+ __u32 minor_ver;
+ /* @patch_ver: patch uc fw version */
+ __u32 patch_ver;
+ /* @branch_ver: branch uc fw version */
+ __u32 branch_ver;
+
+ /** @pad2: MBZ */
+ __u32 pad2;
+
+ /** @reserved: Reserved */
+ __u64 reserved;
+};
+
/**
* DOC: Engine Discovery uAPI
*
base-commit: a42554bf0755b80fdfb8e91ca35ae6835bb3534d
--
2.34.1
Re: [PATCH] drm/i915: Don't set PIPE_CONTROL_FLUSH_L3 for aux inval
Fixes all regressions we saw, I also run some extra vulkan and GL
workloads, no regressions observed.
Tested-by: Tapani Pälli
On 26.9.2023 17.24, Nirmoy Das wrote:
PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation
so don't set that.
Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before invalidation")
Cc: Jonathan Cavitt
Cc: Andi Shyti
Cc: # v5.8+
Cc: Andrzej Hajda
Cc: Tvrtko Ursulin
Cc: Matt Roper
Cc: Tejas Upadhyay
Cc: Lucas De Marchi
Cc: Prathap Kumar Valsan
Cc: Tapani Pälli
Cc: Mark Janes
Cc: Rodrigo Vivi
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 0143445dba83..ba4c2422b340 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -271,8 +271,17 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70))
bit_group_0 |= PIPE_CONTROL_CCS_FLUSH;
+ /*
+* L3 fabric flush is needed for AUX CCS invalidation
+* which happens as part of pipe-control so we can
+* ignore PIPE_CONTROL_FLUSH_L3. Also PIPE_CONTROL_FLUSH_L3
+* deals with Protected Memory which is not needed for
+* AUX CCS invalidation and lead to unwanted side effects.
+*/
+ if (mode & EMIT_FLUSH)
+ bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
+
bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH;
- bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
bit_group_1 |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
/* Wa_1409600907:tgl,adl-p */
Re: [PATCH] drm/gpuvm: doc: fix filename references
On Tue, 26 Sept 2023 at 20:52, Danilo Krummrich wrote:
>
> Commit f72c2db47080 ("drm/gpuvm: rename struct drm_gpuva_manager to
> struct drm_gpuvm") did also change the corresponding filenames which are
> referenced from the documentation, but were not adjusted accordingly.
> Hence, fix up those filenames.
Acked-by: Dave Airlie
>
> Fixes: f72c2db47080 ("drm/gpuvm: rename struct drm_gpuva_manager to struct
> drm_gpuvm")
> Reported-by: Stephen Rothwell
> Closes:
> https://lore.kernel.org/dri-devel/20230926150725.4cca5...@canb.auug.org.au/
> Signed-off-by: Danilo Krummrich
> ---
> Documentation/gpu/drm-mm.rst | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index c19b34b1c0ed..602010cb6894 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -466,40 +466,40 @@ DRM MM Range Allocator Function References
> .. kernel-doc:: drivers/gpu/drm/drm_mm.c
> :export:
>
> -DRM GPU VA Manager
> -==
> +DRM GPUVM
> +=
>
> Overview
>
>
> -.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c
> +.. kernel-doc:: drivers/gpu/drm/drm_gpuvm.c
> :doc: Overview
>
> Split and Merge
> ---
>
> -.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c
> +.. kernel-doc:: drivers/gpu/drm/drm_gpuvm.c
> :doc: Split and Merge
>
> Locking
> ---
>
> -.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c
> +.. kernel-doc:: drivers/gpu/drm/drm_gpuvm.c
> :doc: Locking
>
> Examples
>
>
> -.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c
> +.. kernel-doc:: drivers/gpu/drm/drm_gpuvm.c
> :doc: Examples
>
> -DRM GPU VA Manager Function References
> ---
> +DRM GPUVM Function References
> +-
>
> -.. kernel-doc:: include/drm/drm_gpuva_mgr.h
> +.. kernel-doc:: include/drm/drm_gpuvm.h
> :internal:
>
> -.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c
> +.. kernel-doc:: drivers/gpu/drm/drm_gpuvm.c
> :export:
>
> DRM Buddy Allocator
> --
> 2.41.0
>
Re: [PATCH v4 02/10] drm/sched: Convert drm scheduler to use a work queue rather than kthread
Hi,
On 2023-09-19 01:01, Matthew Brost wrote:
> In XE, the new Intel GPU driver, a choice has made to have a 1 to 1
> mapping between a drm_gpu_scheduler and drm_sched_entity. At first this
> seems a bit odd but let us explain the reasoning below.
>
> 1. In XE the submission order from multiple drm_sched_entity is not
> guaranteed to be the same completion even if targeting the same hardware
> engine. This is because in XE we have a firmware scheduler, the GuC,
> which allowed to reorder, timeslice, and preempt submissions. If a using
> shared drm_gpu_scheduler across multiple drm_sched_entity, the TDR falls
> apart as the TDR expects submission order == completion order. Using a
> dedicated drm_gpu_scheduler per drm_sched_entity solve this problem.
>
> 2. In XE submissions are done via programming a ring buffer (circular
> buffer), a drm_gpu_scheduler provides a limit on number of jobs, if the
> limit of number jobs is set to RING_SIZE / MAX_SIZE_PER_JOB we get flow
> control on the ring for free.
>
> A problem with this design is currently a drm_gpu_scheduler uses a
> kthread for submission / job cleanup. This doesn't scale if a large
> number of drm_gpu_scheduler are used. To work around the scaling issue,
> use a worker rather than kthread for submission / job cleanup.
>
> v2:
> - (Rob Clark) Fix msm build
> - Pass in run work queue
> v3:
> - (Boris) don't have loop in worker
> v4:
> - (Tvrtko) break out submit ready, stop, start helpers into own patch
> v5:
> - (Boris) default to ordered work queue
>
> Signed-off-by: Matthew Brost
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> drivers/gpu/drm/etnaviv/etnaviv_sched.c| 2 +-
> drivers/gpu/drm/lima/lima_sched.c | 2 +-
> drivers/gpu/drm/msm/msm_ringbuffer.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_sched.c| 2 +-
> drivers/gpu/drm/panfrost/panfrost_job.c| 2 +-
> drivers/gpu/drm/scheduler/sched_main.c | 118 ++---
> drivers/gpu/drm/v3d/v3d_sched.c| 10 +-
> include/drm/gpu_scheduler.h| 14 ++-
> 9 files changed, 79 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index e366f61c3aed..16f3cfe1574a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2279,7 +2279,7 @@ static int amdgpu_device_init_schedulers(struct
> amdgpu_device *adev)
> break;
> }
>
> - r = drm_sched_init(&ring->sched, &amdgpu_sched_ops,
> + r = drm_sched_init(&ring->sched, &amdgpu_sched_ops, NULL,
> ring->num_hw_submission, 0,
> timeout, adev->reset_domain->wq,
> ring->sched_score, ring->name,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> index 345fec6cb1a4..618a804ddc34 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> @@ -134,7 +134,7 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
> {
> int ret;
>
> - ret = drm_sched_init(&gpu->sched, &etnaviv_sched_ops,
> + ret = drm_sched_init(&gpu->sched, &etnaviv_sched_ops, NULL,
>etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
>msecs_to_jiffies(500), NULL, NULL,
>dev_name(gpu->dev), gpu->dev);
> diff --git a/drivers/gpu/drm/lima/lima_sched.c
> b/drivers/gpu/drm/lima/lima_sched.c
> index ffd91a5ee299..8d858aed0e56 100644
> --- a/drivers/gpu/drm/lima/lima_sched.c
> +++ b/drivers/gpu/drm/lima/lima_sched.c
> @@ -488,7 +488,7 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe,
> const char *name)
>
> INIT_WORK(&pipe->recover_work, lima_sched_recover_work);
>
> - return drm_sched_init(&pipe->base, &lima_sched_ops, 1,
> + return drm_sched_init(&pipe->base, &lima_sched_ops, NULL, 1,
> lima_job_hang_limit,
> msecs_to_jiffies(timeout), NULL,
> NULL, name, pipe->ldev->dev);
> diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c
> b/drivers/gpu/drm/msm/msm_ringbuffer.c
> index 40c0bc35a44c..b8865e61b40f 100644
> --- a/drivers/gpu/drm/msm/msm_ringbuffer.c
> +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
> @@ -94,7 +94,7 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu
> *gpu, int id,
>/* currently managing hangcheck ourselves: */
> sched_timeout = MAX_SCHEDULE_TIMEOUT;
>
> - ret = drm_sched_init(&ring->sched, &msm_sched_ops,
> + ret = drm_sched_init(&ring->sched, &msm_sched_ops, NULL,
> num_hw_submissions, 0, sched_timeout,
> NULL, NULL, to_msm_bo(ring->bo)->name, gpu->dev->dev);
checkpatch.pl complains here about unmatched open parens.
Re: [PATCH v6 10/16] dt-bindings: media: mediatek: mdp3: add component TDSHP for MT8195
On Sat, 2023-09-23 at 19:34 +0200, Krzysztof Kozlowski wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 22/09/2023 09:21, Moudy Ho wrote: > > Add the fundamental hardware configuration of component TDSHP, > > which is controlled by MDP3 on MT8195. > > > > Signed-off-by: Moudy Ho > > --- > > .../bindings/media/mediatek,mdp3-tdshp.yaml | 61 > +++ > > 1 file changed, 61 insertions(+) > > create mode 100644 > Documentation/devicetree/bindings/media/mediatek,mdp3-tdshp.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3- > tdshp.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3- > tdshp.yaml > > new file mode 100644 > > index ..0ac904cbc2c0 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3- > tdshp.yaml > > @@ -0,0 +1,61 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/mediatek,mdp3-tdshp.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: MediaTek Media Data Path 3 TDSHP > > + > > +maintainers: > > + - Matthias Brugger > > + - Moudy Ho > > + > > +description: > > + One of Media Data Path 3 (MDP3) components used to improve image > > + sharpness and contrast. > > + > > +properties: > > + compatible: > > +enum: > > + - mediatek,mt8195-mdp3-tdshp > > + > > + reg: > > +maxItems: 1 > > + > > + mediatek,gce-client-reg: > > +description: > > + The register of display function block to be set by gce. > There are 4 arguments, > > + such as gce node, subsys id, offset and register size. The > subsys id that is > > + mapping to the register of display function blocks is > defined in the gce header > > + include/dt-bindings/gce/-gce.h of each chips. > > +$ref: /schemas/types.yaml#/definitions/phandle-array > > +items: > > + items: > > +- description: phandle of GCE > > +- description: GCE subsys id > > +- description: register offset > > +- description: register size > > +maxItems: 1 > > + > > + clocks: > > +minItems: 1 > > NAK. So you ignored all the review. Brilliant. > > I am getting fed up with Mediatek's approach. It's not the first > time. > > Best regards, > Krzysztof > Hi Krzysztof, I apologize sincerely for overlooking despite your multiple reminders. To prevent similar incidents, I will ensure a thorough scrutiny of everything in question. I genuinely appreciate your patient review and deeply regret any inconvenience this may have caused you. Sincerely, Moudy
Re: [PATCH 2/2] drm/amd/display: Fix null pointer dereference in error message
On 2023-09-26 01:56, Cong Liu wrote:
This patch fixes a null pointer dereference in the error message that is
printed when the Display Core (DC) fails to initialize. The original
message includes the DC version number, which is undefined if the DC is
not initialized.
Fixes: 9788d087caff ("drm/amd/display: improve the message printed when loading
DC")
Signed-off-by: Cong Liu
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 8e98dda1e084..bf52a909f558 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1703,8 +1703,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
DRM_INFO("Display Core v%s initialized on %s\n", DC_VER,
dce_version_to_string(adev->dm.dc->ctx->dce_version));
} else {
- DRM_INFO("Display Core v%s failed to initialize on %s\n",
DC_VER,
-dce_version_to_string(adev->dm.dc->ctx->dce_version));
+ DRM_INFO("Display Core failed to initialize with v%s!\n",
DC_VER);
There is value in printing the version number. Let's not remove it.
Instead you can probably fix it by doing a NULL check on adev->dm.dc->ctx.
Hi Harry
I don't understand what you mean. Are you saying that I need to add a
NULL check in the if statement (i.e. if(adev->dm.dc &&
adev->dm.dc->ctx)), because adev->dm.dc is NULL in the else statement
and there is no way to print adev->dm.dc->ctx->dce_version.
Regards
Cong
Harry
goto error;
}
Re: [PATCH drm-misc-next 1/3] drm/sched: implement dynamic job flow control
Hi, On 2023-09-26 20:13, Danilo Krummrich wrote: > On 9/26/23 22:43, Luben Tuikov wrote: >> Hi, >> >> On 2023-09-24 18:43, Danilo Krummrich wrote: >>> Currently, job flow control is implemented simply by limiting the amount >>> of jobs in flight. Therefore, a scheduler is initialized with a >>> submission limit that corresponds to a certain amount of jobs. >> >> "certain"? How about this instead: >> " ... that corresponds to the number of jobs which can be sent >>to the hardware."? >> >>> >>> This implies that for each job drivers need to account for the maximum >> ^, >> Please add a comma after "job". >> >>> job size possible in order to not overflow the ring buffer. >> >> Well, different hardware designs would implement this differently. >> Ideally, you only want pointers into the ring buffer, and then >> the hardware consumes as much as it can. But this is a moot point >> and it's always a good idea to have a "job size" hint from the client. >> So this is a good patch. >> >> Ideally, you want to say that the hardware needs to be able to >> accommodate the number of jobs which can fit in the hardware >> queue times the largest job. This is a waste of resources >> however, and it is better to give a hint as to the size of a job, >> by the client. If the hardware can peek and understand dependencies, >> on top of knowing the "size of the job", it can be an extremely >> efficient scheduler. >> >>> >>> However, there are drivers, such as Nouveau, where the job size has a >>> rather large range. For such drivers it can easily happen that job >>> submissions not even filling the ring by 1% can block subsequent >>> submissions, which, in the worst case, can lead to the ring run dry. >>> >>> In order to overcome this issue, allow for tracking the actual job size >>> instead of the amount job jobs. Therefore, add a field to track a job's >> >> "the amount job jobs." --> "the number of jobs." > > Yeah, I somehow manage to always get this wrong, which I guess you noticed > below already. > > That's all good points below - gonna address them. Forgot to mention, title tweak, "implement dynamic job flow control" --> "implement dynamic flow job control" would perhaps be better? Unless that was meant to be "job-flow control"? > Did you see Boris' response regarding a separate callback in order to fetch > the job's submission units dynamically? Since this is needed by PowerVR, I'd > like to include this in V2. What's your take on that? Both of them have good valid points. The whole point is to guarantee forward progress, and to be able to easily debug a stuck driver. Using a fence in prepare-job would be easy to see that the fence never triggered, but if we replace this with dynamic job credits, then debugging would be hard, as Christian pointed out. > My only concern with that would be that if I got what Boris was saying > correctly calling > > WARN_ON(s_job->submission_units > sched->submission_limit); > > from drm_sched_can_queue() wouldn't work anymore, since this could indeed > happen > temporarily. I think this was also Christian's concern. Indeed. We don't want hardware/drivers to game the scheduler, but want to guarantee forward progress, e.g. a job with N number of credits completes, and those credits are added to the available credits, and then a new, smaller or bigger job is accepted for execution (prepare-job, run-job, etc.). Feel free to rename "units" to "credits" as this is what is used in hardware and link protocols, and naturally this is what I'm used to calling such mechanisms. I say, implement it, post the patch, and we'll take a look. It's a good thing and we should definitely develop it. Thanks! -- Regards, Luben
[PATCH 3/3] drm/nouveau: exec: report max pushs through getparam
Report the maximum number of IBs that can be pushed with a single
DRM_IOCTL_NOUVEAU_EXEC through DRM_IOCTL_NOUVEAU_GETPARAM.
While the maximum number of IBs per ring might vary between chipsets,
the kernel will make sure that userspace can only push a fraction of the
maximum number of IBs per ring per job, such that we avoid a situation
where there's only a single job occupying the ring, which could
potentially lead to the ring run dry.
Using DRM_IOCTL_NOUVEAU_GETPARAM to report the maximum number of IBs
that can be pushed with a single DRM_IOCTL_NOUVEAU_EXEC implies that
all channels of a given device have the same ring size.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_abi16.c | 19 +++
drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_dma.h | 3 +++
drivers/gpu/drm/nouveau/nouveau_exec.c | 7 ---
drivers/gpu/drm/nouveau/nouveau_exec.h | 5 +
include/uapi/drm/nouveau_drm.h | 10 ++
6 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c
b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index 30afbec9e3b1..1a198689b391 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -31,6 +31,7 @@
#include "nouveau_drv.h"
#include "nouveau_dma.h"
+#include "nouveau_exec.h"
#include "nouveau_gem.h"
#include "nouveau_chan.h"
#include "nouveau_abi16.h"
@@ -183,6 +184,20 @@ nouveau_abi16_fini(struct nouveau_abi16 *abi16)
cli->abi16 = NULL;
}
+static inline unsigned int
+getparam_dma_ib_max(struct nvif_device *device)
+{
+ const struct nvif_mclass dmas[] = {
+ { NV03_CHANNEL_DMA, 0 },
+ { NV10_CHANNEL_DMA, 0 },
+ { NV17_CHANNEL_DMA, 0 },
+ { NV40_CHANNEL_DMA, 0 },
+ {}
+ };
+
+ return nvif_mclass(&device->object, dmas) < 0 ? NV50_DMA_IB_MAX : 0;
+}
+
int
nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
{
@@ -247,6 +262,10 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
case NOUVEAU_GETPARAM_GRAPH_UNITS:
getparam->value = nvkm_gr_units(gr);
break;
+ case NOUVEAU_GETPARAM_EXEC_PUSH_MAX:
+ getparam->value = getparam_dma_ib_max(device) /
+ NOUVEAU_EXEC_PUSH_MAX_DIV;
+ break;
default:
NV_PRINTK(dbg, cli, "unknown parameter %lld\n",
getparam->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index ac56f4689ee3..c3c2ab887978 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -456,7 +456,7 @@ nouveau_channel_init(struct nouveau_channel *chan, u32
vram, u32 gart)
chan->user_get = 0x44;
chan->user_get_hi = 0x60;
chan->dma.ib_base = 0x1 / 4;
- chan->dma.ib_max = (0x02000 / 8) - 1;
+ chan->dma.ib_max = NV50_DMA_IB_MAX;
chan->dma.ib_put = 0;
chan->dma.ib_free = chan->dma.ib_max - chan->dma.ib_put;
chan->dma.max = chan->dma.ib_base;
diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.h
b/drivers/gpu/drm/nouveau/nouveau_dma.h
index 1744d95b233e..c52cda82353e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dma.h
+++ b/drivers/gpu/drm/nouveau/nouveau_dma.h
@@ -49,6 +49,9 @@ void nv50_dma_push(struct nouveau_channel *, u64 addr, u32
length,
/* Maximum push buffer size. */
#define NV50_DMA_PUSH_MAX_LENGTH 0x7f
+/* Maximum IBs per ring. */
+#define NV50_DMA_IB_MAX ((0x02000 / 8) - 1)
+
/* Object handles - for stuff that's doesn't use handle == oclass. */
enum {
NvDmaFB = 0x8002,
diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c
b/drivers/gpu/drm/nouveau/nouveau_exec.c
index ba6913a3efb6..5b5c4a77b8e6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_exec.c
+++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
@@ -346,7 +346,7 @@ nouveau_exec_ioctl_exec(struct drm_device *dev,
struct nouveau_channel *chan = NULL;
struct nouveau_exec_job_args args = {};
struct drm_nouveau_exec *req = data;
- int ret = 0;
+ int push_max, ret = 0;
if (unlikely(!abi16))
return -ENOMEM;
@@ -371,9 +371,10 @@ nouveau_exec_ioctl_exec(struct drm_device *dev,
if (!chan->dma.ib_max)
return nouveau_abi16_put(abi16, -ENOSYS);
- if (unlikely(req->push_count > NOUVEAU_GEM_MAX_PUSH)) {
+ push_max = chan->dma.ib_max / NOUVEAU_EXEC_PUSH_MAX_DIV;
+ if (unlikely(req->push_count > push_max)) {
NV_PRINTK(err, cli, "pushbuf push count exceeds limit: %d max
%d\n",
-req->push_count, NOUVEAU_GEM_MAX_PUSH);
+ req->push_count, push_max);
return nouveau_abi16_put(abi16, -EINVAL);
}
diff --g
[PATCH 2/3] drm/nouveau: chan: use channel class definitions
Use channel class definitions instead of magic numbers.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_chan.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index dffbee59be6a..ac56f4689ee3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -442,9 +442,11 @@ nouveau_channel_init(struct nouveau_channel *chan, u32
vram, u32 gart)
}
/* initialise dma tracking parameters */
- switch (chan->user.oclass & 0x00ff) {
- case 0x006b:
- case 0x006e:
+ switch (chan->user.oclass) {
+ case NV03_CHANNEL_DMA:
+ case NV10_CHANNEL_DMA:
+ case NV17_CHANNEL_DMA:
+ case NV40_CHANNEL_DMA:
chan->user_put = 0x40;
chan->user_get = 0x44;
chan->dma.max = (0x1 / 4) - 2;
--
2.41.0
[PATCH 1/3] drm/nouveau: chan: use struct nvif_mclass
Use actual struct nvif_mclass instead of identical anonymous struct.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_chan.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index 1fd5ccf41128..dffbee59be6a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -259,10 +259,7 @@ static int
nouveau_channel_ctor(struct nouveau_drm *drm, struct nvif_device *device, bool
priv, u64 runm,
struct nouveau_channel **pchan)
{
- static const struct {
- s32 oclass;
- int version;
- } hosts[] = {
+ const struct nvif_mclass hosts[] = {
{ AMPERE_CHANNEL_GPFIFO_B, 0 },
{ AMPERE_CHANNEL_GPFIFO_A, 0 },
{ TURING_CHANNEL_GPFIFO_A, 0 },
--
2.41.0
Re: [PATCH v4 01/10] drm/sched: Add drm_sched_submit_* helpers
On 2023-09-19 01:01, Matthew Brost wrote:
> Add scheduler submit ready, stop, and start helpers to hide the
> implementation details of the scheduler from the drivers.
>
> Signed-off-by: Matthew Brost
> ---
> .../drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 15 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 12 +++---
> drivers/gpu/drm/msm/adreno/adreno_device.c| 6 ++-
> drivers/gpu/drm/scheduler/sched_main.c| 40 ++-
> include/drm/gpu_scheduler.h | 3 ++
> 6 files changed, 60 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> index 625db444df1c..36a1accbc846 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> @@ -290,7 +290,7 @@ static int suspend_resume_compute_scheduler(struct
> amdgpu_device *adev, bool sus
> for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> struct amdgpu_ring *ring = &adev->gfx.compute_ring[i];
>
> - if (!(ring && ring->sched.thread))
> + if (!(ring && drm_sched_submit_ready(&ring->sched)))
> continue;
>
> /* stop secheduler and drain ring. */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index a4faea4fa0b5..fb5dad687168 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1659,9 +1659,9 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file
> *m, void *unused)
> for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
> struct amdgpu_ring *ring = adev->rings[i];
>
> - if (!ring || !ring->sched.thread)
> + if (!ring || !drm_sched_submit_ready(&ring->sched))
> continue;
> - kthread_park(ring->sched.thread);
> + drm_sched_submit_stop(&ring->sched);
> }
>
> seq_puts(m, "run ib test:\n");
> @@ -1675,9 +1675,9 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file
> *m, void *unused)
> for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
> struct amdgpu_ring *ring = adev->rings[i];
>
> - if (!ring || !ring->sched.thread)
> + if (!ring || !drm_sched_submit_ready(&ring->sched))
> continue;
> - kthread_unpark(ring->sched.thread);
> + drm_sched_submit_start(&ring->sched);
> }
>
> up_write(&adev->reset_domain->sem);
> @@ -1897,7 +1897,8 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64
> val)
>
> ring = adev->rings[val];
>
> - if (!ring || !ring->funcs->preempt_ib || !ring->sched.thread)
> + if (!ring || !ring->funcs->preempt_ib ||
> + !drm_sched_submit_ready(&ring->sched))
> return -EINVAL;
>
> /* the last preemption failed */
> @@ -1915,7 +1916,7 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64
> val)
> goto pro_end;
>
> /* stop the scheduler */
> - kthread_park(ring->sched.thread);
> + drm_sched_submit_stop(&ring->sched);
>
> /* preempt the IB */
> r = amdgpu_ring_preempt_ib(ring);
> @@ -1949,7 +1950,7 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64
> val)
>
> failure:
> /* restart the scheduler */
> - kthread_unpark(ring->sched.thread);
> + drm_sched_submit_start(&ring->sched);
>
> up_read(&adev->reset_domain->sem);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 30c4f5cca02c..e366f61c3aed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4588,7 +4588,7 @@ bool amdgpu_device_has_job_running(struct amdgpu_device
> *adev)
> for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> struct amdgpu_ring *ring = adev->rings[i];
>
> - if (!ring || !ring->sched.thread)
> + if (!ring || !drm_sched_submit_ready(&ring->sched))
> continue;
>
> spin_lock(&ring->sched.job_list_lock);
> @@ -4727,7 +4727,7 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device
> *adev,
> for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> struct amdgpu_ring *ring = adev->rings[i];
>
> - if (!ring || !ring->sched.thread)
> + if (!ring || !drm_sched_submit_ready(&ring->sched))
> continue;
>
> /* Clear job fence from fence drv to avoid force_completion
> @@ -5266,7 +5266,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
> *adev,
> for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> struct amdgpu_ring *ring = tmp_adev->rings[i];
>
> - if (!ring || !ring->sc
Re: [PATCH drm-misc-next 1/3] drm/sched: implement dynamic job flow control
Hi, On 9/27/23 01:48, Luben Tuikov wrote: Hi, Please also CC me to the whole set, as opposed to just one patch of the set. And so in the future. There is no series. I created a series in the first place, but finally decided to send this one and a few driver patches separately. However, I just accidentally sent out the wrong one still containing the "1/3" addition. - Danilo Thanks!
Re: [PATCH drm-misc-next 1/3] drm/sched: implement dynamic job flow control
On 9/26/23 22:43, Luben Tuikov wrote: Hi, On 2023-09-24 18:43, Danilo Krummrich wrote: Currently, job flow control is implemented simply by limiting the amount of jobs in flight. Therefore, a scheduler is initialized with a submission limit that corresponds to a certain amount of jobs. "certain"? How about this instead: " ... that corresponds to the number of jobs which can be sent to the hardware."? This implies that for each job drivers need to account for the maximum ^, Please add a comma after "job". job size possible in order to not overflow the ring buffer. Well, different hardware designs would implement this differently. Ideally, you only want pointers into the ring buffer, and then the hardware consumes as much as it can. But this is a moot point and it's always a good idea to have a "job size" hint from the client. So this is a good patch. Ideally, you want to say that the hardware needs to be able to accommodate the number of jobs which can fit in the hardware queue times the largest job. This is a waste of resources however, and it is better to give a hint as to the size of a job, by the client. If the hardware can peek and understand dependencies, on top of knowing the "size of the job", it can be an extremely efficient scheduler. However, there are drivers, such as Nouveau, where the job size has a rather large range. For such drivers it can easily happen that job submissions not even filling the ring by 1% can block subsequent submissions, which, in the worst case, can lead to the ring run dry. In order to overcome this issue, allow for tracking the actual job size instead of the amount job jobs. Therefore, add a field to track a job's "the amount job jobs." --> "the number of jobs." Yeah, I somehow manage to always get this wrong, which I guess you noticed below already. That's all good points below - gonna address them. Did you see Boris' response regarding a separate callback in order to fetch the job's submission units dynamically? Since this is needed by PowerVR, I'd like to include this in V2. What's your take on that? My only concern with that would be that if I got what Boris was saying correctly calling WARN_ON(s_job->submission_units > sched->submission_limit); from drm_sched_can_queue() wouldn't work anymore, since this could indeed happen temporarily. I think this was also Christian's concern. - Danilo submission units, which represents the amount of units a job contributes to the scheduler's submission limit. Yeah, that's a good thing. I suppose that drivers which don't support this, would just use "1" to achieve the same behaviour as before. Signed-off-by: Danilo Krummrich --- This patch is based on Matt's scheduler work [1]. [1] https://lore.kernel.org/dri-devel/20230919050155.2647172-1-matthew.br...@intel.com/ --- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- drivers/gpu/drm/lima/lima_sched.c | 2 +- drivers/gpu/drm/msm/msm_gem_submit.c | 2 +- drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- .../gpu/drm/scheduler/gpu_scheduler_trace.h | 2 +- drivers/gpu/drm/scheduler/sched_entity.c | 5 +- drivers/gpu/drm/scheduler/sched_main.c| 81 +-- drivers/gpu/drm/v3d/v3d_gem.c | 2 +- include/drm/gpu_scheduler.h | 18 +++-- 11 files changed, 78 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index 78476bc75b4e..d54daaf64bf1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -115,7 +115,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct amdgpu_vm *vm, if (!entity) return 0; - return drm_sched_job_init(&(*job)->base, entity, owner); + return drm_sched_job_init(&(*job)->base, entity, 1, owner); } int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 45403ea38906..74a446711207 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -538,7 +538,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, ret = drm_sched_job_init(&submit->sched_job, &ctx->sched_entity[args->pipe], -submit->ctx); +1, submit->ctx); if (ret) goto err_submit_put; diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c index 50c2075228aa..5dc6678e1eb9 100644 --- a/drivers/gpu/drm/lima/lima_sched.c +++ b/drivers/gpu/drm/lima/lima_sched.c @@ -123,7 +123,7 @@ int lima_sched_task_init(struct lima_sche
Re: [PATCH drm-misc-next 1/3] drm/sched: implement dynamic job flow control
Hi,
Please also CC me to the whole set, as opposed to just one patch of the set.
And so in the future.
Thanks!
--
Regards,
Luben
On 2023-09-26 16:43, Luben Tuikov wrote:
> Hi,
>
> On 2023-09-24 18:43, Danilo Krummrich wrote:
>> Currently, job flow control is implemented simply by limiting the amount
>> of jobs in flight. Therefore, a scheduler is initialized with a
>> submission limit that corresponds to a certain amount of jobs.
>
> "certain"? How about this instead:
> " ... that corresponds to the number of jobs which can be sent
> to the hardware."?
>
>>
>> This implies that for each job drivers need to account for the maximum
> ^,
> Please add a comma after "job".
>
>> job size possible in order to not overflow the ring buffer.
>
> Well, different hardware designs would implement this differently.
> Ideally, you only want pointers into the ring buffer, and then
> the hardware consumes as much as it can. But this is a moot point
> and it's always a good idea to have a "job size" hint from the client.
> So this is a good patch.
>
> Ideally, you want to say that the hardware needs to be able to
> accommodate the number of jobs which can fit in the hardware
> queue times the largest job. This is a waste of resources
> however, and it is better to give a hint as to the size of a job,
> by the client. If the hardware can peek and understand dependencies,
> on top of knowing the "size of the job", it can be an extremely
> efficient scheduler.
>
>>
>> However, there are drivers, such as Nouveau, where the job size has a
>> rather large range. For such drivers it can easily happen that job
>> submissions not even filling the ring by 1% can block subsequent
>> submissions, which, in the worst case, can lead to the ring run dry.
>>
>> In order to overcome this issue, allow for tracking the actual job size
>> instead of the amount job jobs. Therefore, add a field to track a job's
>
> "the amount job jobs." --> "the number of jobs."
>
>> submission units, which represents the amount of units a job contributes
>> to the scheduler's submission limit.
>
> Yeah, that's a good thing.
>
> I suppose that drivers which don't support this, would just use "1" to achieve
> the same behaviour as before.
>
>>
>> Signed-off-by: Danilo Krummrich
>> ---
>> This patch is based on Matt's scheduler work [1].
>>
>> [1]
>> https://lore.kernel.org/dri-devel/20230919050155.2647172-1-matthew.br...@intel.com/
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +-
>> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
>> drivers/gpu/drm/lima/lima_sched.c | 2 +-
>> drivers/gpu/drm/msm/msm_gem_submit.c | 2 +-
>> drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +-
>> drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +-
>> .../gpu/drm/scheduler/gpu_scheduler_trace.h | 2 +-
>> drivers/gpu/drm/scheduler/sched_entity.c | 5 +-
>> drivers/gpu/drm/scheduler/sched_main.c| 81 +--
>> drivers/gpu/drm/v3d/v3d_gem.c | 2 +-
>> include/drm/gpu_scheduler.h | 18 +++--
>> 11 files changed, 78 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> index 78476bc75b4e..d54daaf64bf1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> @@ -115,7 +115,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct
>> amdgpu_vm *vm,
>> if (!entity)
>> return 0;
>>
>> -return drm_sched_job_init(&(*job)->base, entity, owner);
>> +return drm_sched_job_init(&(*job)->base, entity, 1, owner);
>> }
>>
>> int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev,
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> index 45403ea38906..74a446711207 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> @@ -538,7 +538,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev,
>> void *data,
>>
>> ret = drm_sched_job_init(&submit->sched_job,
>> &ctx->sched_entity[args->pipe],
>> - submit->ctx);
>> + 1, submit->ctx);
>> if (ret)
>> goto err_submit_put;
>>
>> diff --git a/drivers/gpu/drm/lima/lima_sched.c
>> b/drivers/gpu/drm/lima/lima_sched.c
>> index 50c2075228aa..5dc6678e1eb9 100644
>> --- a/drivers/gpu/drm/lima/lima_sched.c
>> +++ b/drivers/gpu/drm/lima/lima_sched.c
>> @@ -123,7 +123,7 @@ int lima_sched_task_init(struct lima_sched_task *task,
>> for (i = 0; i < num_bos; i++)
>> drm_gem_object_get(&bos[i]->base.base);
>>
>> -err = drm_sched_job_init(&task->base, &context->base, vm);
>> +err = drm_sched_job_init(&task->base, &context->base, 1, vm);
>> if (err) {
>> kfre
Re: [PATCH v2] drm/i915/mtl: Skip MCR ops for ring fault register
On Tue, Sep 26, 2023 at 11:58:02PM +0200, Nirmoy Das wrote:
> On MTL GEN12_RING_FAULT_REG is not replicated so don't
> do mcr based operation for this register.
>
> v2: use MEDIA_VER() instead of GRAPHICS_VER()(Matt).
>
> Signed-off-by: Nirmoy Das
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 13 -
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 +
> drivers/gpu/drm/i915/i915_gpu_error.c | 10 +-
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 93062c35e072..430738607f61 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -262,10 +262,21 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
> I915_MASTER_ERROR_INTERRUPT);
> }
>
> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> + /*
> + * for media tile this ring fault register is not replicated
Nitpicks: s/tile/gt/ and either write it as "For the media GT..."
(singular) or "For media GTs..." (plural). Same with the other copy of
this comment farther down.
> + * so skip doing mcr ops on it.
> + */
> + if (MEDIA_VER(i915) == 13 && gt->type == GT_MEDIA) {
I guess for now we should probably make this (and the one farther down)
a ">=" instead of "==" under the assumption future media versions will
do the same in case we get some kind of refresh platform down the road
with a slightly higher version number.
Aside from those minor tweaks,
Reviewed-by: Matt Roper
> + intel_uncore_rmw(uncore, XELPMP_RING_FAULT_REG,
> + RING_FAULT_VALID, 0);
> + intel_uncore_posting_read(uncore,
> + XELPMP_RING_FAULT_REG);
> +
> + } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> intel_gt_mcr_multicast_rmw(gt, XEHP_RING_FAULT_REG,
> RING_FAULT_VALID, 0);
> intel_gt_mcr_read_any(gt, XEHP_RING_FAULT_REG);
> +
> } else if (GRAPHICS_VER(i915) >= 12) {
> intel_uncore_rmw(uncore, GEN12_RING_FAULT_REG,
> RING_FAULT_VALID, 0);
> intel_uncore_posting_read(uncore, GEN12_RING_FAULT_REG);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index cca4bac8f8b0..eecd0a87a647 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1084,6 +1084,7 @@
>
> #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> #define XEHP_RING_FAULT_REG MCR_REG(0xcec4)
> +#define XELPMP_RING_FAULT_REG_MMIO(0xcec4)
> #define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
> #define RING_FAULT_GTTSEL_MASK (1 << 11)
> #define RING_FAULT_SRCID(x)(((x) >> 3) & 0xff)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index f4ebcfb70289..f0b691ea3a6e 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1234,7 +1234,15 @@ static void engine_record_registers(struct
> intel_engine_coredump *ee)
> if (GRAPHICS_VER(i915) >= 6) {
> ee->rc_psmi = ENGINE_READ(engine, RING_PSMI_CTL);
>
> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
> + /*
> + * for media tile this ring fault register is not replicated
> + * so skip doing mcr ops on it.
> + */
> + if (MEDIA_VER(i915) == 13 && engine->gt->type == GT_MEDIA)
> + ee->fault_reg = intel_uncore_read(engine->uncore,
> +
> XELPMP_RING_FAULT_REG);
> +
> + else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
> ee->fault_reg = intel_gt_mcr_read_any(engine->gt,
>
> XEHP_RING_FAULT_REG);
> else if (GRAPHICS_VER(i915) >= 12)
> --
> 2.41.0
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
[PATCH v2] drm/i915/mtl: Skip MCR ops for ring fault register
On MTL GEN12_RING_FAULT_REG is not replicated so don't
do mcr based operation for this register.
v2: use MEDIA_VER() instead of GRAPHICS_VER()(Matt).
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/gt/intel_gt.c | 13 -
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 +
drivers/gpu/drm/i915/i915_gpu_error.c | 10 +-
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 93062c35e072..430738607f61 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -262,10 +262,21 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
I915_MASTER_ERROR_INTERRUPT);
}
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
+ /*
+* for media tile this ring fault register is not replicated
+* so skip doing mcr ops on it.
+*/
+ if (MEDIA_VER(i915) == 13 && gt->type == GT_MEDIA) {
+ intel_uncore_rmw(uncore, XELPMP_RING_FAULT_REG,
+RING_FAULT_VALID, 0);
+ intel_uncore_posting_read(uncore,
+ XELPMP_RING_FAULT_REG);
+
+ } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
intel_gt_mcr_multicast_rmw(gt, XEHP_RING_FAULT_REG,
RING_FAULT_VALID, 0);
intel_gt_mcr_read_any(gt, XEHP_RING_FAULT_REG);
+
} else if (GRAPHICS_VER(i915) >= 12) {
intel_uncore_rmw(uncore, GEN12_RING_FAULT_REG,
RING_FAULT_VALID, 0);
intel_uncore_posting_read(uncore, GEN12_RING_FAULT_REG);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index cca4bac8f8b0..eecd0a87a647 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1084,6 +1084,7 @@
#define GEN12_RING_FAULT_REG _MMIO(0xcec4)
#define XEHP_RING_FAULT_REGMCR_REG(0xcec4)
+#define XELPMP_RING_FAULT_REG _MMIO(0xcec4)
#define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
#define RING_FAULT_GTTSEL_MASK (1 << 11)
#define RING_FAULT_SRCID(x) (((x) >> 3) & 0xff)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index f4ebcfb70289..f0b691ea3a6e 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1234,7 +1234,15 @@ static void engine_record_registers(struct
intel_engine_coredump *ee)
if (GRAPHICS_VER(i915) >= 6) {
ee->rc_psmi = ENGINE_READ(engine, RING_PSMI_CTL);
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
+ /*
+* for media tile this ring fault register is not replicated
+* so skip doing mcr ops on it.
+*/
+ if (MEDIA_VER(i915) == 13 && engine->gt->type == GT_MEDIA)
+ ee->fault_reg = intel_uncore_read(engine->uncore,
+
XELPMP_RING_FAULT_REG);
+
+ else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
ee->fault_reg = intel_gt_mcr_read_any(engine->gt,
XEHP_RING_FAULT_REG);
else if (GRAPHICS_VER(i915) >= 12)
--
2.41.0
Re: [PATCH v3 0/4] Fix up the boe-tv101wum-nl6 panel driver
Hi, On Mon, Sep 18, 2023 at 9:19 AM Doug Anderson wrote: > > Hi, > > On Mon, Jul 3, 2023 at 6:21 AM Linus Walleij wrote: > > > > This is two patches fixing things I would normally complain about > > in reviews, but alas I missed this one, so I go in and fix it up > > myself. > > > > Discovering that a completely unrelated driver has been merged > > into this panel driver I had to bite the bullet and break it out. > > I am pretty suspicious of the other recently added panel as well. > > > > I am surprised that contributors from manufacturers do not seem > > to have datasheets for the display controllers embedded in the > > panels of their products. Can you take a second look? > > > > Signed-off-by: Linus Walleij > > --- > > Changes in v3: > > - Rebase on drm-misc-next > > - Convert the two newly added Starry panels as well. > > - Break out the obvious ILI9882t-based panel into its own driver. > > - Link to v2: > > https://lore.kernel.org/r/20230615-fix-boe-tv101wum-nl6-v2-0-457d7ece4...@linaro.org > > > > Changes in v2: > > - Fix a missed static keyword > > - Link to v1: > > https://lore.kernel.org/r/20230615-fix-boe-tv101wum-nl6-v1-0-8ac378405...@linaro.org > > > > --- > > Linus Walleij (4): > > drm/panel: boe-tv101wum-nl6: Drop macros and open code sequences > > drm/panel: boe-tv101wum-nl6: Drop surplus prepare tracking > > drm/panel: ili9882t: Break out as separate driver > > drm/panel: ili9882t: Break out function for switching page > > > > drivers/gpu/drm/panel/Kconfig |9 + > > drivers/gpu/drm/panel/Makefile |1 + > > drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 3037 > > ++-- > > drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 759 ++ > > 4 files changed, 2067 insertions(+), 1739 deletions(-) > > I'm curious what the latest on this patch series is. Is it abandoned, > or is it still on your list to move forward with it? If it's > abandoned, does that mean we've abandoned the idea of breaking > ili9882t into a separate driver? > > From looking at things that have landed downstream in the ChromeOS > kernel trees it looks as if additional fixes are getting blocked from > being posted/landed because of the limbo state that this is in. I presume Linus is busy or otherwise indisposed. So I guess we have two options here: a) Cong Yang can post any relevant fixes to the existing "monolithic" panel driver so that we can get them landed and at least get things fixed. - or - b) Cong Yang could take over all or some of Linus's series and post new versions of it, addressing feedback. I would tend to say we should go with "a)" because I think Linus needs to be involved in some of the cleanup discussions. -Doug
Re: [PATCH drm-misc-next 1/3] drm/sched: implement dynamic job flow control
Hi,
On 2023-09-24 18:43, Danilo Krummrich wrote:
> Currently, job flow control is implemented simply by limiting the amount
> of jobs in flight. Therefore, a scheduler is initialized with a
> submission limit that corresponds to a certain amount of jobs.
"certain"? How about this instead:
" ... that corresponds to the number of jobs which can be sent
to the hardware."?
>
> This implies that for each job drivers need to account for the maximum
^,
Please add a comma after "job".
> job size possible in order to not overflow the ring buffer.
Well, different hardware designs would implement this differently.
Ideally, you only want pointers into the ring buffer, and then
the hardware consumes as much as it can. But this is a moot point
and it's always a good idea to have a "job size" hint from the client.
So this is a good patch.
Ideally, you want to say that the hardware needs to be able to
accommodate the number of jobs which can fit in the hardware
queue times the largest job. This is a waste of resources
however, and it is better to give a hint as to the size of a job,
by the client. If the hardware can peek and understand dependencies,
on top of knowing the "size of the job", it can be an extremely
efficient scheduler.
>
> However, there are drivers, such as Nouveau, where the job size has a
> rather large range. For such drivers it can easily happen that job
> submissions not even filling the ring by 1% can block subsequent
> submissions, which, in the worst case, can lead to the ring run dry.
>
> In order to overcome this issue, allow for tracking the actual job size
> instead of the amount job jobs. Therefore, add a field to track a job's
"the amount job jobs." --> "the number of jobs."
> submission units, which represents the amount of units a job contributes
> to the scheduler's submission limit.
Yeah, that's a good thing.
I suppose that drivers which don't support this, would just use "1" to achieve
the same behaviour as before.
>
> Signed-off-by: Danilo Krummrich
> ---
> This patch is based on Matt's scheduler work [1].
>
> [1]
> https://lore.kernel.org/dri-devel/20230919050155.2647172-1-matthew.br...@intel.com/
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +-
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
> drivers/gpu/drm/lima/lima_sched.c | 2 +-
> drivers/gpu/drm/msm/msm_gem_submit.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +-
> drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +-
> .../gpu/drm/scheduler/gpu_scheduler_trace.h | 2 +-
> drivers/gpu/drm/scheduler/sched_entity.c | 5 +-
> drivers/gpu/drm/scheduler/sched_main.c| 81 +--
> drivers/gpu/drm/v3d/v3d_gem.c | 2 +-
> include/drm/gpu_scheduler.h | 18 +++--
> 11 files changed, 78 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 78476bc75b4e..d54daaf64bf1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -115,7 +115,7 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct
> amdgpu_vm *vm,
> if (!entity)
> return 0;
>
> - return drm_sched_job_init(&(*job)->base, entity, owner);
> + return drm_sched_job_init(&(*job)->base, entity, 1, owner);
> }
>
> int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 45403ea38906..74a446711207 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -538,7 +538,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void
> *data,
>
> ret = drm_sched_job_init(&submit->sched_job,
>&ctx->sched_entity[args->pipe],
> - submit->ctx);
> + 1, submit->ctx);
> if (ret)
> goto err_submit_put;
>
> diff --git a/drivers/gpu/drm/lima/lima_sched.c
> b/drivers/gpu/drm/lima/lima_sched.c
> index 50c2075228aa..5dc6678e1eb9 100644
> --- a/drivers/gpu/drm/lima/lima_sched.c
> +++ b/drivers/gpu/drm/lima/lima_sched.c
> @@ -123,7 +123,7 @@ int lima_sched_task_init(struct lima_sched_task *task,
> for (i = 0; i < num_bos; i++)
> drm_gem_object_get(&bos[i]->base.base);
>
> - err = drm_sched_job_init(&task->base, &context->base, vm);
> + err = drm_sched_job_init(&task->base, &context->base, 1, vm);
> if (err) {
> kfree(task->bos);
> return err;
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 3f1aa4de3b87..6d230c38e4f5 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -48,7 +48,7 @@ static struct
Re: [PATCH 07/15] mailbox: mediatek: Add loop pkt flag and irq handling for loop command
On 26/09/2023 05:20, Jason-JH Lin (林睿祥) wrote: mdq_pkt_finialize_loop() at [PATCH 8/15]. >>> >>> mtk-cmdq-helper.c and mtk-cmdq-mailbox.c are not in the >>> same maintainer's tree, so I separate this to another patch from >> [PATCH >>> 8/15]. >> >> Why? Anyway it has to go through same tree. You have dependencies. >> Such >> artificial split makes it only difficult to review and understand. >> Re-organize your patchset to be correctly split per each logical >> feature/change. Split per subsystems is not the same. >> > > Yes, these related files are in the different maintainer's tree. > Refer to https://www.kernel.org/doc/linux/MAINTAINERS > > MAILBOX API > M: Jassi Brar > F: drivers/mailbox/ > - drivers/mailbox/mtk-cmdq-mailbox.c > - drivers/mailbox/mtk-cmdq-sec- > mailbox.c > > ARM/Mediatek SoC support > M: Matthias Brugger > F: drivers/soc/mediatek/ > K: mediatek > - drivers/soc/mediatek/mtk-cmdq-helper.c > - > include/linux/soc/mediatek/mtk-cmdq.h > > I think we should add a new MAINTAINER label for mediatek CMDQ mailbox > and put these files together, such as "MAILBOX ARM MHUv2" and "QUALCOM > IPCC MAILBOX DRIVER". Why? It's not related to the topic of splitting patchset into patches. There is no problem of patchsets touching multiple subsystems. We already solved this problem many years ago... > But I don't know how to make a request for that. Anyway, you would not be a maintainer taking patches, just a reviewer called "M:" here... > > Anyway, I'll squash this logical feature to the same patch, no matter > these files are not in the same tree. > Best regards, Krzysztof
Re: [PATCH 5/6] drm/msm/dpu: Add hw revision 4.1 (SDM670)
On 26.09.2023 01:26, Richard Acayan wrote:
> The Snapdragon 670 uses similar clocks (with one frequency added) to the
> Snapdragon 845 but reports DPU revision 4.1. Add support for this DPU
> with configuration from the Pixel 3a downstream kernel.
>
> Since revision 4.0 is SDM845, reuse some configuration from its catalog
> entry.
>
> Link:
> https://android.googlesource.com/kernel/msm/+/368478b0ae76566927a2769a2bf24dfe7f38bb78/arch/arm64/boot/dts/qcom/sdm670-sde.dtsi
> Signed-off-by: Richard Acayan
> ---
> .../msm/disp/dpu1/catalog/dpu_4_1_sdm670.h| 105 ++
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 6 +
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
> 4 files changed, 113 insertions(+)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_1_sdm670.h
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_1_sdm670.h
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_1_sdm670.h
> new file mode 100644
> index ..eaccb16b5db9
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_1_sdm670.h
> @@ -0,0 +1,105 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023, Richard Acayan. All rights reserved.
> + */
> +
> +#ifndef _DPU_4_1_SDM670_H
> +#define _DPU_4_1_SDM670_H
> +
> +static const struct dpu_mdp_cfg sdm670_mdp = {
> + .name = "top_0",
> + .base = 0x0, .len = 0x45c,
> + .features = BIT(DPU_MDP_AUDIO_SELECT),
> + .clk_ctrls = {
> + [DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0},
space before the closing curly bracket, please
[...]
> +
> +static struct dpu_dsc_cfg sdm670_dsc[] = {
const
Konrad
Re: [PATCH v2 1/1] drm/msm/adreno: Add support for SM7150 SoC machine
On 26.09.2023 21:10, Danila Tikhonov wrote: > > I think you mean by name downstream dt - sdmmagpie-gpu.dtsi > > You can see the forked version of the mainline here: > https://github.com/sm7150-mainline/linux/blob/next/arch/arm64/boot/dts/qcom/sm7150.dtsi > > All fdt that we got here, if it is useful for you: > https://github.com/sm7150-mainline/downstream-fdt > > Best wishes, Danila Taking a look at downstream, atoll.dtsi (SC7180) includes sdmmagpie-gpu.dtsi. Bottom line is, they share the speed bins, so it should be fine to just extend the existing entry. Konrad
Re: [PATCH] drm/i915: Don't set PIPE_CONTROL_FLUSH_L3 for aux inval
Hi Nirmoy,
...
> > PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation
> > so don't set that.
> >
> > Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before
> > invalidation")
> > Cc: Jonathan Cavitt
> > Cc: Andi Shyti
> > Cc: # v5.8+
> > Cc: Andrzej Hajda
> > Cc: Tvrtko Ursulin
> > Cc: Matt Roper
> > Cc: Tejas Upadhyay
> > Cc: Lucas De Marchi
> > Cc: Prathap Kumar Valsan
> > Cc: Tapani Pälli
> > Cc: Mark Janes
> > Cc: Rodrigo Vivi
> > Signed-off-by: Nirmoy Das
>
> looks better :)
this was supposed to be:
Reviewed-by: Andi Shyti
Andi
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Skip MCR ops for ring fault register
Hi Matt,
On 9/26/2023 4:38 PM, Matt Roper wrote:
On Tue, Sep 26, 2023 at 04:18:42PM +0200, Nirmoy Das wrote:
On MTL GEN12_RING_FAULT_REG is not replicated so don't
do mcr based operation for this register.
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/gt/intel_gt.c | 14 +-
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 +
drivers/gpu/drm/i915/i915_gpu_error.c | 11 ++-
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 93062c35e072..d4de692e8be1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -262,10 +262,22 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
I915_MASTER_ERROR_INTERRUPT);
}
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
+ /*
+* for media tile this ring fault register is not replicated
+* so skip doing mcr ops on it.
+*/
+ if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50) &&
This should be checking the media version rather than the graphics
version. I.e., "MEDIA_VER(i915) > 13" since it's possible future
versions of the media IP may change the behavior (independently of the
graphics IP versions).
Sounds good. I will replace this with
if (MEDIA_VER(i915) == 13 && engine->gt->type == GT_MEDIA)
to limit this this change on media IP 13
Thanks,
Nirmoy
Matt
+ gt->type == GT_MEDIA) {
+ intel_uncore_rmw(uncore, XELPMP_RING_FAULT_REG,
+RING_FAULT_VALID, 0);
+ intel_uncore_posting_read(uncore,
+ XELPMP_RING_FAULT_REG);
+
+ } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
intel_gt_mcr_multicast_rmw(gt, XEHP_RING_FAULT_REG,
RING_FAULT_VALID, 0);
intel_gt_mcr_read_any(gt, XEHP_RING_FAULT_REG);
+
} else if (GRAPHICS_VER(i915) >= 12) {
intel_uncore_rmw(uncore, GEN12_RING_FAULT_REG,
RING_FAULT_VALID, 0);
intel_uncore_posting_read(uncore, GEN12_RING_FAULT_REG);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index cca4bac8f8b0..eecd0a87a647 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1084,6 +1084,7 @@
#define GEN12_RING_FAULT_REG _MMIO(0xcec4)
#define XEHP_RING_FAULT_REG MCR_REG(0xcec4)
+#define XELPMP_RING_FAULT_REG _MMIO(0xcec4)
#define GEN8_RING_FAULT_ENGINE_ID(x)(((x) >> 12) & 0x7)
#define RING_FAULT_GTTSEL_MASK (1 << 11)
#define RING_FAULT_SRCID(x) (((x) >> 3) & 0xff)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index f4ebcfb70289..83f1a729da8b 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1234,7 +1234,16 @@ static void engine_record_registers(struct
intel_engine_coredump *ee)
if (GRAPHICS_VER(i915) >= 6) {
ee->rc_psmi = ENGINE_READ(engine, RING_PSMI_CTL);
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
+ /*
+* for media tile this ring fault register is not replicated
+* so skip doing mcr ops on it.
+*/
+ if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50) &&
+ engine->gt->type == GT_MEDIA)
+ ee->fault_reg = intel_uncore_read(engine->uncore,
+
XELPMP_RING_FAULT_REG);
+
+ else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
ee->fault_reg = intel_gt_mcr_read_any(engine->gt,
XEHP_RING_FAULT_REG);
else if (GRAPHICS_VER(i915) >= 12)
--
2.41.0
Re: [PATCH v2 1/1] drm/msm/adreno: Add support for SM7150 SoC machine
I think you mean by name downstream dt - sdmmagpie-gpu.dtsi You can see the forked version of the mainline here: https://github.com/sm7150-mainline/linux/blob/next/arch/arm64/boot/dts/qcom/sm7150.dtsi All fdt that we got here, if it is useful for you: https://github.com/sm7150-mainline/downstream-fdt Best wishes, Danila On 26.09.2023 20:40, Konrad Dybcio wrote: > On 26.09.2023 19:42, Danila Tikhonov wrote: > > SM7150 has 5 power levels which correspond to 5 speed-bin values: 0, > > 128, 146, 167, 172. Speed-bin value is calulated as FMAX/4.8MHz round up > > to zero decimal places. > > > > Also a618 on SM7150 uses a615 zapfw. Add a squashed version (.mbn). > > > > Add this as machine = "qcom,sm7150", because speed-bin values are > > different from atoll (sc7180/sm7125). > > > > Signed-off-by: Danila Tikhonov < dan...@jiaxyga.com > > > --- > What's the downstream dt name for 7150? > > Do you have some more complete tree published somewhere? > > Konrad
[PATCH v2 1/1] drm/msm/adreno: Add support for SM7150 SoC machine
SM7150 has 5 power levels which correspond to 5 speed-bin values: 0,
128, 146, 167, 172. Speed-bin value is calulated as FMAX/4.8MHz round up
to zero decimal places.
Also a618 on SM7150 uses a615 zapfw. Add a squashed version (.mbn).
Add this as machine = "qcom,sm7150", because speed-bin values are
different from atoll (sc7180/sm7125).
Signed-off-by: Danila Tikhonov
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index fa527935ffd4..cb2f459cbcc4 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -293,6 +293,27 @@ static const struct adreno_info gpulist[] = {
{ 157, 3 },
{ 127, 4 },
),
+ }, {
+ .machine = "qcom,sm7150",
+ .chip_ids = ADRENO_CHIP_IDS(0x06010800),
+ .family = ADRENO_6XX_GEN1,
+ .fw = {
+ [ADRENO_FW_SQE] = "a630_sqe.fw",
+ [ADRENO_FW_GMU] = "a630_gmu.bin",
+ },
+ .gmem = SZ_512K,
+ .inactive_period = DRM_MSM_INACTIVE_PERIOD,
+ .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
+ .init = a6xx_gpu_init,
+ .zapfw = "a615_zap.mbn",
+ .hwcg = a615_hwcg,
+ .speedbins = ADRENO_SPEEDBINS(
+ { 0, 0 },
+ { 128, 1 },
+ { 146, 2 },
+ { 167, 3 },
+ { 172, 4 },
+ ),
}, {
.chip_ids = ADRENO_CHIP_IDS(0x06010800),
.family = ADRENO_6XX_GEN1,
@@ -507,6 +528,7 @@ MODULE_FIRMWARE("qcom/a530_zap.b00");
MODULE_FIRMWARE("qcom/a530_zap.b01");
MODULE_FIRMWARE("qcom/a530_zap.b02");
MODULE_FIRMWARE("qcom/a540_gpmu.fw2");
+MODULE_FIRMWARE("qcom/a615_zap.mbn");
MODULE_FIRMWARE("qcom/a619_gmu.bin");
MODULE_FIRMWARE("qcom/a630_sqe.fw");
MODULE_FIRMWARE("qcom/a630_gmu.bin");
--
2.34.1
Re: [PATCH v3] drm/dp_mst: Fix NULL deref in get_mst_branch_device_by_guid_helper()
On Fri, Sep 22, 2023 at 8:34 AM Lukasz Majczak wrote:
>
> As drm_dp_get_mst_branch_device_by_guid() is called from
> drm_dp_get_mst_branch_device_by_guid(), mstb parameter has to be checked,
> otherwise NULL dereference may occur in the call to
> the memcpy() and cause following:
>
> [12579.365869] BUG: kernel NULL pointer dereference, address: 0049
> [12579.365878] #PF: supervisor read access in kernel mode
> [12579.365880] #PF: error_code(0x) - not-present page
> [12579.365882] PGD 0 P4D 0
> [12579.365887] Oops: [#1] PREEMPT SMP NOPTI
> ...
> [12579.365895] Workqueue: events_long drm_dp_mst_up_req_work
> [12579.365899] RIP: 0010:memcmp+0xb/0x29
> [12579.365921] Call Trace:
> [12579.365927] get_mst_branch_device_by_guid_helper+0x22/0x64
> [12579.365930] drm_dp_mst_up_req_work+0x137/0x416
> [12579.365933] process_one_work+0x1d0/0x419
> [12579.365935] worker_thread+0x11a/0x289
> [12579.365938] kthread+0x13e/0x14f
> [12579.365941] ? process_one_work+0x419/0x419
> [12579.365943] ? kthread_blkcg+0x31/0x31
> [12579.365946] ret_from_fork+0x1f/0x30
>
> As get_mst_branch_device_by_guid_helper() is recursive, moving condition
> to the first line allow to remove a similar one for step over of NULL elements
> inside a loop.
>
> Fixes: 5e93b8208d3c ("drm/dp/mst: move GUID storage from mgr, port to only
> mst branch")
> Cc: # 4.14+
> Signed-off-by: Lukasz Majczak
> ---
>
> v2->v3:
> * Fixed patch description.
>
> drivers/gpu/drm/display/drm_dp_mst_topology.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index ed96cfcfa304..8c929ef72c72 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -2574,14 +2574,14 @@ static struct drm_dp_mst_branch
> *get_mst_branch_device_by_guid_helper(
> struct drm_dp_mst_branch *found_mstb;
> struct drm_dp_mst_port *port;
>
> + if (!mstb)
> + return NULL;
> +
> if (memcmp(mstb->guid, guid, 16) == 0)
> return mstb;
>
>
> list_for_each_entry(port, &mstb->ports, next) {
> - if (!port->mstb)
> - continue;
> -
> found_mstb = get_mst_branch_device_by_guid_helper(port->mstb,
> guid);
>
> if (found_mstb)
> --
> 2.42.0.515.g380fc7ccd1-goog
>
Reviewed-by: Radoslaw Biernacki
[PATCH] drm/atomic: Perform blocking commits on workqueue
From: Ray Strode A drm atomic commit can be quite slow on some hardware. It can lead to a lengthy queue of commands that need to get processed and waited on before control can go back to user space. If user space is a real-time thread, that delay can have severe consequences, leading to the process getting killed for exceeding rlimits. This commit addresses the problem by always running the slow part of a commit on a workqueue, separated from the task initiating the commit. This change makes the nonblocking and blocking paths work in the same way, and as a result allows the task to sleep and not use up its RLIMIT_RTTIME allocation. Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2861 Signed-off-by: Ray Strode --- drivers/gpu/drm/drm_atomic_helper.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 292e38eb6218..1a1e68d98d38 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2028,64 +2028,63 @@ int drm_atomic_helper_commit(struct drm_device *dev, * This is the point of no return - everything below never fails except * when the hw goes bonghits. Which means we can commit the new state on * the software side now. */ ret = drm_atomic_helper_swap_state(state, true); if (ret) goto err; /* * Everything below can be run asynchronously without the need to grab * any modeset locks at all under one condition: It must be guaranteed * that the asynchronous work has either been cancelled (if the driver * supports it, which at least requires that the framebuffers get * cleaned up with drm_atomic_helper_cleanup_planes()) or completed * before the new state gets committed on the software side with * drm_atomic_helper_swap_state(). * * This scheme allows new atomic state updates to be prepared and * checked in parallel to the asynchronous completion of the previous * update. Which is important since compositors need to figure out the * composition of the next frame right after having submitted the * current layout. * * NOTE: Commit work has multiple phases, first hardware commit, then * cleanup. We want them to overlap, hence need system_unbound_wq to * make sure work items don't artificially stall on each another. */ drm_atomic_state_get(state); - if (nonblock) - queue_work(system_unbound_wq, &state->commit_work); - else - commit_tail(state); + queue_work(system_unbound_wq, &state->commit_work); + if (!nonblock) + flush_work(&state->commit_work); return 0; err: drm_atomic_helper_cleanup_planes(dev, state); return ret; } EXPORT_SYMBOL(drm_atomic_helper_commit); /** * DOC: implementing nonblocking commit * * Nonblocking atomic commits should use struct &drm_crtc_commit to sequence * different operations against each another. Locks, especially struct * &drm_modeset_lock, should not be held in worker threads or any other * asynchronous context used to commit the hardware state. * * drm_atomic_helper_commit() implements the recommended sequence for * nonblocking commits, using drm_atomic_helper_setup_commit() internally: * * 1. Run drm_atomic_helper_prepare_planes(). Since this can fail and we * need to propagate out of memory/VRAM errors to userspace, it must be called * synchronously. * * 2. Synchronize with any outstanding nonblocking commit worker threads which * might be affected by the new state update. This is handled by * drm_atomic_helper_setup_commit(). * * Asynchronous workers need to have sufficient parallelism to be able to run * different atomic commits on different CRTCs in parallel. The simplest way to -- 2.41.0
[PATCH v2 0/1] drm/msm/adreno: Add support for SM7150
This patch adds support for SM7150 SoC machine. Changes in v2: - Use a630_gmu.bin instead of a618_gmu.bin. - Use squashed version of a615_zap (.mbn). - Drop .revn. - Link to v1: https://lore.kernel.org/all/20230913191957.26537-1-dan...@jiaxyga.com/ Danila Tikhonov (1): drm/msm/adreno: Add support for SM7150 SoC machine drivers/gpu/drm/msm/adreno/adreno_device.c | 22 ++ 1 file changed, 22 insertions(+) -- 2.34.1
Re: [PATCH v4 0/3] drm/bridge: tfp410: Add i2c support
How do I bump this patch submission?
[PATCH v4 3/3] drm/i915/gt: Timeout when waiting for idle in suspending
When suspending, add a timeout when calling
intel_gt_pm_wait_for_idle else if we have a lost
G2H event that holds a wakeref (which would be
indicative of a bug elsewhere in the driver),
driver will at least complete the suspend-resume
cycle, (albeit not hitting all the targets for
low power hw counters), instead of hanging in the kernel.
Signed-off-by: Alan Previn
Reviewed-by: Rodrigo Vivi
Tested-by: Mousumi Jana
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 6 +-
drivers/gpu/drm/i915/gt/intel_gt_pm.h | 7 ++-
drivers/gpu/drm/i915/intel_wakeref.c | 14 ++
drivers/gpu/drm/i915/intel_wakeref.h | 6 --
5 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 84a75c95f3f7..9c6151b78e1d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -687,7 +687,7 @@ void intel_engines_release(struct intel_gt *gt)
if (!engine->release)
continue;
- intel_wakeref_wait_for_idle(&engine->wakeref);
+ intel_wakeref_wait_for_idle(&engine->wakeref, 0);
GEM_BUG_ON(intel_engine_pm_is_awake(engine));
engine->release(engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 59b5658a17fb..820217c06dc7 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -289,6 +289,7 @@ int intel_gt_resume(struct intel_gt *gt)
static void wait_for_suspend(struct intel_gt *gt)
{
+ int timeout_ms = CONFIG_DRM_I915_MAX_REQUEST_BUSYWAIT ? : 1;
/*
* On rare occasions, we've observed the fence completion trigger
* free_engines asynchronously via rcu_call. Ensure those are done.
@@ -308,7 +309,10 @@ static void wait_for_suspend(struct intel_gt *gt)
intel_gt_retire_requests(gt);
}
- intel_gt_pm_wait_for_idle(gt);
+ /* we are suspending, so we shouldn't be waiting forever */
+ if (intel_gt_pm_wait_timeout_for_idle(gt, timeout_ms) == -ETIMEDOUT)
+ gt_warn(gt, "bailing from %s after %d milisec timeout\n",
+ __func__, timeout_ms);
}
void intel_gt_suspend_prepare(struct intel_gt *gt)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
index 6c9a46452364..5358acc2b5b1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
@@ -68,7 +68,12 @@ static inline void intel_gt_pm_might_put(struct intel_gt *gt)
static inline int intel_gt_pm_wait_for_idle(struct intel_gt *gt)
{
- return intel_wakeref_wait_for_idle(>->wakeref);
+ return intel_wakeref_wait_for_idle(>->wakeref, 0);
+}
+
+static inline int intel_gt_pm_wait_timeout_for_idle(struct intel_gt *gt, int
timeout_ms)
+{
+ return intel_wakeref_wait_for_idle(>->wakeref, timeout_ms);
}
void intel_gt_pm_init_early(struct intel_gt *gt);
diff --git a/drivers/gpu/drm/i915/intel_wakeref.c
b/drivers/gpu/drm/i915/intel_wakeref.c
index 718f2f1b6174..383a37521415 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.c
+++ b/drivers/gpu/drm/i915/intel_wakeref.c
@@ -111,14 +111,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
"wakeref.work", &key->work, 0);
}
-int intel_wakeref_wait_for_idle(struct intel_wakeref *wf)
+int intel_wakeref_wait_for_idle(struct intel_wakeref *wf, int timeout_ms)
{
- int err;
+ int err = 0;
might_sleep();
- err = wait_var_event_killable(&wf->wakeref,
- !intel_wakeref_is_active(wf));
+ if (!timeout_ms)
+ err = wait_var_event_killable(&wf->wakeref,
+ !intel_wakeref_is_active(wf));
+ else if (wait_var_event_timeout(&wf->wakeref,
+ !intel_wakeref_is_active(wf),
+ msecs_to_jiffies(timeout_ms)) < 1)
+ err = -ETIMEDOUT;
+
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/intel_wakeref.h
b/drivers/gpu/drm/i915/intel_wakeref.h
index ec881b097368..302694a780d2 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.h
+++ b/drivers/gpu/drm/i915/intel_wakeref.h
@@ -251,15 +251,17 @@ __intel_wakeref_defer_park(struct intel_wakeref *wf)
/**
* intel_wakeref_wait_for_idle: Wait until the wakeref is idle
* @wf: the wakeref
+ * @timeout_ms: Timeout in ms, 0 means never timeout.
*
* Wait for the earlier asynchronous release of the wakeref. Note
* this will wait for any third party as well, so make sure you only wait
* when you have control over the wakeref and trust no one else is acquiring
* it.
*
- * Return: 0 on success, error code if killed.
+ * Returns 0 on su
[PATCH v4 0/3] Resolve suspend-resume racing with GuC destroy-context-worker
This series is the result of debugging issues root caused to races between the GuC's destroyed_worker_func being triggered vs repeating suspend-resume cycles with concurrent delayed fence signals for engine-freeing. The reproduction steps require that an app is launched right before the start of the suspend cycle where it creates a new gem context and submits a tiny workload that would complete in the middle of the suspend cycle. However this app uses dma-buffer sharing or dma-fence with non-GPU objects or signals that eventually triggers a FENCE_FREE via__i915_sw_fence_notify that connects to engines_notify -> free_engines_rcu -> intel_context_put -> kref_put(&ce->ref..) that queues the worker after the GuCs CTB has been disabled (i.e. after i915-gem's suspend-late). This sequence is a corner-case and required repeating this app->suspend->resume cycle ~1500 times across 4 identical systems to see it once. That said, based on above callstack, it is clear that merely flushing the context destruction worker, which is obviously missing and needed, isn't sufficient. Because of that, this series adds additional patches besides the obvious (Patch #1) flushing of the worker during the suspend flows. It also includes (Patch #2) closing a race between sending the context-deregistration H2G vs the CTB getting disabled in the midst of it (by detecing the failure and unrolling the guc-lrc-unpin flow) and (Patch #32) not infinitely waiting in intel_gt_pm_wait_timeout_for_idle when in the suspend-flow. NOTE: We are still observing one more wakeref leak from gt but not necessarilty guc. We are still debugging this so this series will very likely get rev'd up again. Changes from prior revs: v3: - In Patch #3, when deregister_context fails, instead of calling intel_gt_pm_put(that might sleep), call __intel_wakeref_put (without ASYNC flag) (Rodrigo/Anshuman). - In wait_for_suspend add an rcu_barrier before we proceed to wait for idle. (Anshuman) v2: - Patch #2 Restructure code in guc_lrc_desc_unpin so it's more readible to differentiate (1)direct guc-id cleanup ..vs (2) sending the H2G ctx-destroy action .. vs (3) the unrolling steps if the H2G fails. - Patch #2 Add a check to close the race sooner by checking for intel_guc_is_ready from destroyed_worker_func. - Patch #2 When guc_submission_send_busy_loop gets a failure from intel_guc_send_busy_loop, we need to undo i.e. decrement the outstanding_submission_g2h. - Patch #3 In wait_for_suspend, fix checking of return from intel_gt_pm_wait_timeout_for_idle to now use -ETIMEDOUT and add documentation for intel_wakeref_wait_for_idle. (Rodrigo). Alan Previn (3): drm/i915/guc: Flush context destruction worker at suspend drm/i915/guc: Close deregister-context race against CT-loss drm/i915/gt: Timeout when waiting for idle in suspending drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 13 ++- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 7 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 86 --- .../gpu/drm/i915/gt/uc/intel_guc_submission.h | 2 + drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 + drivers/gpu/drm/i915/intel_wakeref.c | 14 ++- drivers/gpu/drm/i915/intel_wakeref.h | 6 +- 8 files changed, 112 insertions(+), 20 deletions(-) base-commit: a42554bf0755b80fdfb8e91ca35ae6835bb3534d -- 2.39.0
[PATCH v4 2/3] drm/i915/guc: Close deregister-context race against CT-loss
If we are at the end of suspend or very early in resume
its possible an async fence signal (via rcu_call) is triggered
to free_engines which could lead us to the execution of
the context destruction worker (after a prior worker flush).
Thus, when suspending, insert an rcu_barrier at the start
of wait_for_suspend so that all such cases have completed
and context destruction list isn't missing anything.
In destroyed_worker_func, close the race against CT-loss
by checking that CT is enabled before calling into
deregister_destroyed_contexts.
Based on testing, guc_lrc_desc_unpin may still race and fail
as we traverse the GuC's context-destroy list because the
CT could be disabled right before calling GuC's CT send function.
We've witnessed this race condition once every ~6000-8000
suspend-resume cycles while ensuring workloads that render
something onscreen is continuously started just before
we suspend (and the workload is small enough to complete
and trigger the queued engine/context free-up either very
late in suspend or very early in resume).
In such a case, we need to unroll the entire process because
guc-lrc-unpin takes a gt wakeref which only gets released in
the G2H IRQ reply that never comes through in this corner
case. Without the unroll, the taken wakeref is leaked and will
cascade into a kernel hang later at the tail end of suspend in
this function:
intel_wakeref_wait_for_idle(>->wakeref)
(called by) - intel_gt_pm_wait_for_idle
(called by) - wait_for_suspend
Thus, do an unroll in guc_lrc_desc_unpin and deregister_destroyed_-
contexts if guc_lrc_desc_unpin fails due to CT send falure.
When unrolling, keep the context in the GuC's destroy-list so
it can get picked up on the next destroy worker invocation
(if suspend aborted) or get fully purged as part of a GuC
sanitization (end of suspend) or a reset flow.
Signed-off-by: Alan Previn
Signed-off-by: Anshuman Gupta
Tested-by: Mousumi Jana
---
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 7 ++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 81 ---
2 files changed, 77 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 5a942af0a14e..59b5658a17fb 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -289,6 +289,13 @@ int intel_gt_resume(struct intel_gt *gt)
static void wait_for_suspend(struct intel_gt *gt)
{
+ /*
+* On rare occasions, we've observed the fence completion trigger
+* free_engines asynchronously via rcu_call. Ensure those are done.
+* This path is only called on suspend, so it's an acceptable cost.
+*/
+ rcu_barrier();
+
if (!intel_gt_pm_is_awake(gt))
return;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index fdd7179f502a..465baf7660d7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -235,6 +235,13 @@ set_context_destroyed(struct intel_context *ce)
ce->guc_state.sched_state |= SCHED_STATE_DESTROYED;
}
+static inline void
+clr_context_destroyed(struct intel_context *ce)
+{
+ lockdep_assert_held(&ce->guc_state.lock);
+ ce->guc_state.sched_state &= ~SCHED_STATE_DESTROYED;
+}
+
static inline bool context_pending_disable(struct intel_context *ce)
{
return ce->guc_state.sched_state & SCHED_STATE_PENDING_DISABLE;
@@ -612,6 +619,8 @@ static int guc_submission_send_busy_loop(struct intel_guc
*guc,
u32 g2h_len_dw,
bool loop)
{
+ int ret;
+
/*
* We always loop when a send requires a reply (i.e. g2h_len_dw > 0),
* so we don't handle the case where we don't get a reply because we
@@ -622,7 +631,11 @@ static int guc_submission_send_busy_loop(struct intel_guc
*guc,
if (g2h_len_dw)
atomic_inc(&guc->outstanding_submission_g2h);
- return intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
+ ret = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
+ if (ret)
+ atomic_dec(&guc->outstanding_submission_g2h);
+
+ return ret;
}
int intel_guc_wait_for_pending_msg(struct intel_guc *guc,
@@ -3205,12 +3218,13 @@ static void guc_context_close(struct intel_context *ce)
spin_unlock_irqrestore(&ce->guc_state.lock, flags);
}
-static inline void guc_lrc_desc_unpin(struct intel_context *ce)
+static inline int guc_lrc_desc_unpin(struct intel_context *ce)
{
struct intel_guc *guc = ce_to_guc(ce);
struct intel_gt *gt = guc_to_gt(guc);
unsigned long flags;
bool disabled;
+ int ret;
GEM_BUG_ON(!intel_gt_pm_is_awake(gt));
GEM_BUG_ON(!ctx_id_mapped(guc, ce->guc_id.id));
@@ -3220,19 +3234,38 @@ static inline vo
[PATCH v4 1/3] drm/i915/guc: Flush context destruction worker at suspend
When suspending, flush the context-guc-id
deregistration worker at the final stages of
intel_gt_suspend_late when we finally call gt_sanitize
that eventually leads down to __uc_sanitize so that
the deregistration worker doesn't fire off later as
we reset the GuC microcontroller.
Signed-off-by: Alan Previn
Reviewed-by: Rodrigo Vivi
Tested-by: Mousumi Jana
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 5 +
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h | 2 ++
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 ++
3 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index ae3495a9c814..fdd7179f502a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1610,6 +1610,11 @@ static void guc_flush_submissions(struct intel_guc *guc)
spin_unlock_irqrestore(&sched_engine->lock, flags);
}
+void intel_guc_submission_flush_work(struct intel_guc *guc)
+{
+ flush_work(&guc->submission_state.destroyed_worker);
+}
+
static void guc_flush_destroyed_contexts(struct intel_guc *guc);
void intel_guc_submission_reset_prepare(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index c57b29cdb1a6..b6df75622d3b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -38,6 +38,8 @@ int intel_guc_wait_for_pending_msg(struct intel_guc *guc,
bool interruptible,
long timeout);
+void intel_guc_submission_flush_work(struct intel_guc *guc);
+
static inline bool intel_guc_submission_is_supported(struct intel_guc *guc)
{
return guc->submission_supported;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 98b103375b7a..eb3554cb5ea4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -693,6 +693,8 @@ void intel_uc_suspend(struct intel_uc *uc)
return;
}
+ intel_guc_submission_flush_work(guc);
+
with_intel_runtime_pm(&uc_to_gt(uc)->i915->runtime_pm, wakeref) {
err = intel_guc_suspend(guc);
if (err)
--
2.39.0
Re: [PATCH v2 1/1] drm/msm/adreno: Add support for SM7150 SoC machine
On 26.09.2023 19:42, Danila Tikhonov wrote: > SM7150 has 5 power levels which correspond to 5 speed-bin values: 0, > 128, 146, 167, 172. Speed-bin value is calulated as FMAX/4.8MHz round up > to zero decimal places. > > Also a618 on SM7150 uses a615 zapfw. Add a squashed version (.mbn). > > Add this as machine = "qcom,sm7150", because speed-bin values are > different from atoll (sc7180/sm7125). > > Signed-off-by: Danila Tikhonov > --- What's the downstream dt name for 7150? Do you have some more complete tree published somewhere? Konrad
Re: [PATCH 4/7] arm64: dts: qcom: sc7280: Add ZAP shader support
On 26.09.2023 20:24, Konrad Dybcio wrote:
> Non-Chrome SC7280-family platforms ship a ZAP shader with the Adreno GPU.
> Describe that and make sure it doesn't interfere with Chrome devices.
>
> Signed-off-by: Konrad Dybcio
> ---
> arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 2 ++
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 10 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> index 5d462ae14ba1..88fc67c3646e 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> @@ -17,6 +17,8 @@
> * required by the setup for Chrome boards.
> */
>
> +/delete-node/ &gpu_zap_mem;
> +/delete-node/ &gpu_zap_shader;
> /delete-node/ &hyp_mem;
> /delete-node/ &xbl_mem;
> /delete-node/ &reserved_xbl_uefi_log;
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 66f1eb83cca7..c38ddf267ef5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -152,6 +152,11 @@ ipa_fw_mem: memory@8b70 {
> no-map;
> };
>
> + gpu_zap_mem: zap@8b71a000 {
> + reg = <0 0x8b71a000 0 0x2000>;
> + no-map;
> + };
> +
> rmtfs_mem: memory@9c90 {
> compatible = "qcom,rmtfs-mem";
> reg = <0x0 0x9c90 0x0 0x28>;
> @@ -2608,6 +2613,11 @@ gpu: gpu@3d0 {
> nvmem-cells = <&gpu_speed_bin>;
> nvmem-cell-names = "speed_bin";
>
> + gpu_zap_shader: zap-shader {
> + memory-region = <&gpu_zap_mem>;
> + firmware-name = "qcom/a660_zap.mdt";
Gah. This line shouldn't have been there.
Considering it's not the only oops, I'll resend.
Konrad
Re: [PATCH 6/7] arm64: dts: qcom: sc7280: Mark Adreno SMMU as DMA coherent
On 26.09.2023 20:24, Konrad Dybcio wrote:
> The SMMUs on sc7280 are cache-coherent. APPS_SMMU is marked as such,
> mark the GPU one as well.
>
> Signed-off-by: Konrad Dybcio
> ---
Fixes: 96c471970b7b ("arm64: dts: qcom: sc7280: Add gpu support")
Sorry.
Konrad
[PATCH 6/7] arm64: dts: qcom: sc7280: Mark Adreno SMMU as DMA coherent
The SMMUs on sc7280 are cache-coherent. APPS_SMMU is marked as such,
mark the GPU one as well.
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 0d96d1454c49..edaca6c2cf8c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2783,6 +2783,7 @@ adreno_smmu: iommu@3da {
"gpu_cc_hub_aon_clk";
power-domains = <&gpucc GPU_CC_CX_GDSC>;
+ dma-coherent;
};
remoteproc_mpss: remoteproc@408 {
--
2.42.0
[PATCH 7/7] arm64: dts: qcom: sc7280: Add 0xac Adreno speed bin
A643 (A635 speedbin 0xac) tops out at 812 MHz. Fill in the
opp-supported-hw appropriately.
Note that fuseval 0xac is referred to as speedbin 1 downstream, but
that was already in use upstream, so 2 was chosen instead.
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index edaca6c2cf8c..ccc2dd6c45de 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2626,14 +2626,14 @@ opp-31500 {
opp-hz = /bits/ 64 <31500>;
opp-level =
;
opp-peak-kBps = <1804000>;
- opp-supported-hw = <0x03>;
+ opp-supported-hw = <0x07>;
};
opp-45000 {
opp-hz = /bits/ 64 <45000>;
opp-level = ;
opp-peak-kBps = <4068000>;
- opp-supported-hw = <0x03>;
+ opp-supported-hw = <0x07>;
};
/* Only applicable for SKUs which has 550Mhz as
Fmax */
@@ -2648,28 +2648,28 @@ opp-55000-1 {
opp-hz = /bits/ 64 <55000>;
opp-level =
;
opp-peak-kBps = <6832000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-60800 {
opp-hz = /bits/ 64 <60800>;
opp-level =
;
opp-peak-kBps = <8368000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-7 {
opp-hz = /bits/ 64 <7>;
opp-level = ;
opp-peak-kBps = <8532000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-81200 {
opp-hz = /bits/ 64 <81200>;
opp-level =
;
opp-peak-kBps = <8532000>;
- opp-supported-hw = <0x02>;
+ opp-supported-hw = <0x06>;
};
opp-84000 {
--
2.42.0
[PATCH 5/7] arm64: dts: qcom: sc7280: Fix up GPU SIDs
GPU_SMMU SID 1 is meant for Adreno LPAC (Low Priority Async Compute).
On platforms that support it (in firmware), it is necessary to
describe that link, or Adreno register access will hang the board.
Add that and fix up the SMR mask of SID 0, which seems to have been
copypasted from another SoC.
Fixes: 96c471970b7b ("arm64: dts: qcom: sc7280: Add gpu support")
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index c38ddf267ef5..0d96d1454c49 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2603,7 +2603,8 @@ gpu: gpu@3d0 {
"cx_mem",
"cx_dbgc";
interrupts = ;
- iommus = <&adreno_smmu 0 0x401>;
+ iommus = <&adreno_smmu 0 0x400>,
+<&adreno_smmu 1 0x400>;
operating-points-v2 = <&gpu_opp_table>;
qcom,gmu = <&gmu>;
interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt
SLAVE_EBI1 0>;
--
2.42.0
[PATCH 3/7] drm/msm/adreno: Add A635 speedbin 0xac (A643)
Downstream calls this the "speedbin 1", but that number is already
occupied. Use index two.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 16527fe8584d..4977fd759b5b 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -460,6 +460,7 @@ static const struct adreno_info gpulist[] = {
.speedbins = ADRENO_SPEEDBINS(
{ 0, 0 },
{ 117, 0 },
+ { 172, 2 }, /* Called speedbin 1 downstream, but let's
not break things! */
{ 190, 1 },
),
}, {
--
2.42.0
[PATCH 4/7] arm64: dts: qcom: sc7280: Add ZAP shader support
Non-Chrome SC7280-family platforms ship a ZAP shader with the Adreno GPU.
Describe that and make sure it doesn't interfere with Chrome devices.
Signed-off-by: Konrad Dybcio
---
arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 2 ++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 10 ++
2 files changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index 5d462ae14ba1..88fc67c3646e 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -17,6 +17,8 @@
* required by the setup for Chrome boards.
*/
+/delete-node/ &gpu_zap_mem;
+/delete-node/ &gpu_zap_shader;
/delete-node/ &hyp_mem;
/delete-node/ &xbl_mem;
/delete-node/ &reserved_xbl_uefi_log;
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 66f1eb83cca7..c38ddf267ef5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -152,6 +152,11 @@ ipa_fw_mem: memory@8b70 {
no-map;
};
+ gpu_zap_mem: zap@8b71a000 {
+ reg = <0 0x8b71a000 0 0x2000>;
+ no-map;
+ };
+
rmtfs_mem: memory@9c90 {
compatible = "qcom,rmtfs-mem";
reg = <0x0 0x9c90 0x0 0x28>;
@@ -2608,6 +2613,11 @@ gpu: gpu@3d0 {
nvmem-cells = <&gpu_speed_bin>;
nvmem-cell-names = "speed_bin";
+ gpu_zap_shader: zap-shader {
+ memory-region = <&gpu_zap_mem>;
+ firmware-name = "qcom/a660_zap.mdt";
+ };
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.42.0
[PATCH 1/7] drm/msm/a6xx: Fix unknown speedbin case
When opp-supported-hw is present under an OPP node, but no form of
opp_set_supported_hw() has been called, that OPP is ignored by the API
and marked as unsupported.
Before Commit c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to
device table"), an unknown speedbin would result in marking all OPPs
as available, but it's better to avoid potentially overclocking the
silicon - the GMU will simply refuse to power up the chip.
Currently, the Adreno speedbin code does just that (AND returns an
invalid error, (int)UINT_MAX). Fix that by defaulting to speedbin 0
(which is conveniently always bound to fuseval == 0).
Fixes: c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to device table")
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index d4e85e24002f..522ca7fe6762 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2237,7 +2237,7 @@ static int a6xx_set_supported_hw(struct device *dev,
const struct adreno_info *i
DRM_DEV_ERROR(dev,
"missing support for speed-bin: %u. Some OPPs may not
be supported by hardware\n",
speedbin);
- return UINT_MAX;
+ supp_hw = BIT(0); /* Default */
}
ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
--
2.42.0
[PATCH 2/7] drm/msm/adreno: Add ZAP firmware name to A635
Some (many?) devices with A635 expect a ZAP shader to be loaded.
Set the file name to allow for that.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index fa527935ffd4..16527fe8584d 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -454,6 +454,7 @@ static const struct adreno_info gpulist[] = {
.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
ADRENO_QUIRK_HAS_HW_APRIV,
.init = a6xx_gpu_init,
+ .zapfw = "a660_zap.mbn",
.hwcg = a660_hwcg,
.address_space_size = SZ_16G,
.speedbins = ADRENO_SPEEDBINS(
--
2.42.0
[PATCH 0/7] Adreno 643 + fixes
as it says on the can drm/msm patches for Rob arm64 patches for linux-arm-msm for use with https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25408 tested on QCM6490 (SC7280-IOT) Fairphone FP5 Signed-off-by: Konrad Dybcio --- Konrad Dybcio (7): drm/msm/a6xx: Fix unknown speedbin case drm/msm/adreno: Add ZAP firmware name to A635 drm/msm/adreno: Add A635 speedbin 0xac (A643) arm64: dts: qcom: sc7280: Add ZAP shader support arm64: dts: qcom: sc7280: Fix up GPU SIDs arm64: dts: qcom: sc7280: Mark Adreno SMMU as DMA coherent arm64: dts: qcom: sc7280: Add 0xac Adreno speed bin arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 2 ++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 26 -- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/adreno_device.c | 2 ++ 4 files changed, 24 insertions(+), 8 deletions(-) --- base-commit: 4ae73bba62a367f2314f6ce69e3085a941983d8b change-id: 20230926-topic-a643-a7ec9a08a3a1 Best regards, -- Konrad Dybcio
RE: [PATCH 5/6] drm/edid: add helpers to get/set struct cea_sad from/to 3-byte sad
> -Original Message-
> From: Nikula, Jani
> Sent: 07 September 2023 14:58
> To: dri-devel@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org; Nikula, Jani ;
> Golani, Mitulkumar Ajitkumar
> Subject: [PATCH 5/6] drm/edid: add helpers to get/set struct cea_sad
> from/to 3-byte sad
>
> Add helpers to pack/unpack SADs. Both ways and non-static, as follow-up
> work needs them.
>
> Cc: Mitul Golani
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/drm_edid.c | 33 -
> drivers/gpu/drm/drm_internal.h | 6 ++
> 2 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index fcdc2c314cde..1260e2688bd7 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5499,6 +5499,27 @@ static void clear_eld(struct drm_connector
> *connector)
> connector->audio_latency[1] = 0;
> }
>
> +/*
> + * Get 3-byte SAD buf from struct cea_sad.
> + */
Just a small nit-pick: 'SAD buffer'.
Otherwise the change looks good to me.
Reviewed-by: Mitul Golani
> +void drm_edid_cta_sad_get(const struct cea_sad *cta_sad, u8 *sad) {
> + sad[0] = cta_sad->format << 3 | cta_sad->channels;
> + sad[1] = cta_sad->freq;
> + sad[2] = cta_sad->byte2;
> +}
> +
> +/*
> + * Set struct cea_sad from 3-byte SAD buf.
> + */
> +void drm_edid_cta_sad_set(struct cea_sad *cta_sad, const u8 *sad) {
> + cta_sad->format = (sad[0] & 0x78) >> 3;
> + cta_sad->channels = sad[0] & 0x07;
> + cta_sad->freq = sad[1] & 0x7f;
> + cta_sad->byte2 = sad[2];
> +}
> +
> /*
> * drm_edid_to_eld - build ELD from EDID
> * @connector: connector corresponding to the HDMI/DP sink @@ -5593,21
> +5614,15 @@ static int _drm_edid_to_sad(const struct drm_edid
> *drm_edid,
> cea_db_iter_for_each(db, &iter) {
> if (cea_db_tag(db) == CTA_DB_AUDIO) {
> struct cea_sad *sads;
> - int j;
> + int i;
>
> count = cea_db_payload_len(db) / 3; /* SAD is 3B */
> sads = kcalloc(count, sizeof(*sads), GFP_KERNEL);
> *psads = sads;
> if (!sads)
> return -ENOMEM;
> - for (j = 0; j < count; j++) {
> - const u8 *sad = &db->data[j * 3];
> -
> - sads[j].format = (sad[0] & 0x78) >> 3;
> - sads[j].channels = sad[0] & 0x7;
> - sads[j].freq = sad[1] & 0x7F;
> - sads[j].byte2 = sad[2];
> - }
> + for (i = 0; i < count; i++)
> + drm_edid_cta_sad_set(&sads[i], &db->data[i
> * 3]);
> break;
> }
> }
> diff --git a/drivers/gpu/drm/drm_internal.h
> b/drivers/gpu/drm/drm_internal.h index ab9a472f4a47..5b7c661da459
> 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -22,6 +22,7 @@
> */
>
> #include
> +#include
>
> #include
> #include
> @@ -31,6 +32,7 @@
>
> #define DRM_IF_VERSION(maj, min) (maj << 16 | min)
>
> +struct cea_sad;
> struct dentry;
> struct dma_buf;
> struct iosys_map;
> @@ -265,3 +267,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev,
> void *data, void drm_framebuffer_print_info(struct drm_printer *p,
> unsigned int indent,
> const struct drm_framebuffer *fb);
> void drm_framebuffer_debugfs_init(struct drm_device *dev);
> +
> +/* drm_edid.c */
> +void drm_edid_cta_sad_get(const struct cea_sad *cta_sad, u8 *sad); void
> +drm_edid_cta_sad_set(struct cea_sad *cta_sad, const u8 *sad);
> --
> 2.39.2
RE: [PATCH 6/6] drm/eld: add helpers to modify the SADs of an ELD
> -Original Message-
> From: Nikula, Jani
> Sent: 07 September 2023 14:58
> To: dri-devel@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org; Nikula, Jani ;
> Golani, Mitulkumar Ajitkumar
> Subject: [PATCH 6/6] drm/eld: add helpers to modify the SADs of an ELD
>
> Occasionally it's necessary for drivers to modify the SADs of an ELD, but it's
> not so cool to have drivers poke at the ELD buffer directly.
>
> Using the helpers to translate between 3-byte SAD and struct cea_sad, add
> ELD helpers to get/set the SADs from/to an ELD.
>
> Cc: Mitul Golani
> Signed-off-by: Jani Nikula
> ---
> Documentation/gpu/drm-kms-helpers.rst | 3 ++
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_eld.c | 55 +++
> include/drm/drm_eld.h | 5 +++
> 4 files changed, 64 insertions(+)
> create mode 100644 drivers/gpu/drm/drm_eld.c
>
> diff --git a/Documentation/gpu/drm-kms-helpers.rst
> b/Documentation/gpu/drm-kms-helpers.rst
> index f0f93aa62545..df91b7cd992e 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -366,6 +366,9 @@ EDID Helper Functions Reference .. kernel-doc::
> include/drm/drm_eld.h
> :internal:
>
> +.. kernel-doc:: drivers/gpu/drm/drm_eld.c
> + :export:
> +
> SCDC Helper Functions Reference
> ===
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index
> 215e78e79125..632e74d823e8 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -22,6 +22,7 @@ drm-y := \
> drm_drv.o \
> drm_dumb_buffers.o \
> drm_edid.o \
> + drm_eld.o \
> drm_encoder.o \
> drm_file.o \
> drm_fourcc.o \
> diff --git a/drivers/gpu/drm/drm_eld.c b/drivers/gpu/drm/drm_eld.c new
> file mode 100644 index ..34e0d71c3550
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_eld.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#include
> +#include
> +
> +#include "drm_internal.h"
> +
> +/**
> + * drm_eld_sad_get - get SAD from ELD to struct cea_sad
> + * @eld: ELD buffer
> + * @i: SAD number
> + * @cta_sad: destination struct cea_sad
> + *
> + * @return: 0 on success, or negative on errors */ int
> +drm_eld_sad_get(const u8 *eld, int i, struct cea_sad *cta_sad) {
Could we use a more descriptive variable name than 'i' for better code
readability in the functions drm_eld_sad_get and drm_eld_sad_set?
possibly something like, `sad_number` or `index` ?
Regards,
Mitul
> + const u8 *sad;
> +
> + if (i >= drm_eld_sad_count(eld))
> + return -EINVAL;
> +
> + sad = eld + DRM_ELD_CEA_SAD(drm_eld_mnl(eld), i);
> +
> + drm_edid_cta_sad_set(cta_sad, sad);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_eld_sad_get);
> +
> +/**
> + * drm_eld_sad_set - set SAD to ELD from struct cea_sad
> + * @eld: ELD buffer
> + * @i: SAD number
> + * @cta_sad: source struct cea_sad
> + *
> + * @return: 0 on success, or negative on errors */ int
> +drm_eld_sad_set(u8 *eld, int i, const struct cea_sad *cta_sad) {
> + u8 *sad;
> +
> + if (i >= drm_eld_sad_count(eld))
> + return -EINVAL;
> +
> + sad = eld + DRM_ELD_CEA_SAD(drm_eld_mnl(eld), i);
> +
> + drm_edid_cta_sad_get(cta_sad, sad);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_eld_sad_set);
> diff --git a/include/drm/drm_eld.h b/include/drm/drm_eld.h index
> 7b674256b9aa..5b320157684c 100644
> --- a/include/drm/drm_eld.h
> +++ b/include/drm/drm_eld.h
> @@ -8,6 +8,8 @@
>
> #include
>
> +struct cea_sad;
> +
> /* ELD Header Block */
> #define DRM_ELD_HEADER_BLOCK_SIZE4
>
> @@ -75,6 +77,9 @@ static inline int drm_eld_mnl(const u8 *eld)
> return (eld[DRM_ELD_CEA_EDID_VER_MNL] &
> DRM_ELD_MNL_MASK) >> DRM_ELD_MNL_SHIFT; }
>
> +int drm_eld_sad_get(const u8 *eld, int i, struct cea_sad *cta_sad); int
> +drm_eld_sad_set(u8 *eld, int i, const struct cea_sad *cta_sad);
> +
> /**
> * drm_eld_sad - Get ELD SAD structures.
> * @eld: pointer to an eld memory structure with sad_count set
> --
> 2.39.2
[PATCH v2] drm/ttm: Make sure the mapped tt pages are decrypted when needed
From: Zack Rusin
Some drivers require the mapped tt pages to be decrypted. In an ideal
world this would have been handled by the dma layer, but the TTM page
fault handling would have to be rewritten to able to do that.
A side-effect of the TTM page fault handling is using a dma allocation
per order (via ttm_pool_alloc_page) which makes it impossible to just
trivially use dma_mmap_attrs. As a result ttm has to be very careful
about trying to make its pgprot for the mapped tt pages match what
the dma layer thinks it is. At the ttm layer it's possible to
deduce the requirement to have tt pages decrypted by checking
whether coherent dma allocations have been requested and the system
is running with confidential computing technologies.
This approach isn't ideal but keeping TTM matching DMAs expectations
for the page properties is in general fragile, unfortunately proper
fix would require a rewrite of TTM's page fault handling.
Fixes vmwgfx with SEV enabled.
v2: Explicitly include cc_platform.h
Signed-off-by: Zack Rusin
Fixes: 3bf3710e3718 ("drm/ttm: Add a generic TTM memcpy move for page-based
iomem")
Cc: Christian König
Cc: Thomas Hellström
Cc: Huang Rui
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc: # v5.14+
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 13 +++--
drivers/gpu/drm/ttm/ttm_tt.c | 8
include/drm/ttm/ttm_tt.h | 9 -
3 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index fd9fd3d15101..0b3f4267130c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -294,7 +294,13 @@ pgprot_t ttm_io_prot(struct ttm_buffer_object *bo, struct
ttm_resource *res,
enum ttm_caching caching;
man = ttm_manager_type(bo->bdev, res->mem_type);
- caching = man->use_tt ? bo->ttm->caching : res->bus.caching;
+ if (man->use_tt) {
+ caching = bo->ttm->caching;
+ if (bo->ttm->page_flags & TTM_TT_FLAG_DECRYPTED)
+ tmp = pgprot_decrypted(tmp);
+ } else {
+ caching = res->bus.caching;
+ }
return ttm_prot_from_caching(caching, tmp);
}
@@ -337,6 +343,8 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo,
.no_wait_gpu = false
};
struct ttm_tt *ttm = bo->ttm;
+ struct ttm_resource_manager *man =
+ ttm_manager_type(bo->bdev, bo->resource->mem_type);
pgprot_t prot;
int ret;
@@ -346,7 +354,8 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo,
if (ret)
return ret;
- if (num_pages == 1 && ttm->caching == ttm_cached) {
+ if (num_pages == 1 && ttm->caching == ttm_cached &&
+ !(man->use_tt && (ttm->page_flags & TTM_TT_FLAG_DECRYPTED))) {
/*
* We're mapping a single page, and the desired
* page protection is consistent with the bo.
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index e0a77671edd6..e4966e2c988d 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -31,6 +31,7 @@
#define pr_fmt(fmt) "[TTM] " fmt
+#include
#include
#include
#include
@@ -81,6 +82,13 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool
zero_alloc)
pr_err("Illegal buffer object type\n");
return -EINVAL;
}
+ /*
+* When using dma_alloc_coherent with memory encryption the
+* mapped TT pages need to be decrypted or otherwise the drivers
+* will end up sending encrypted mem to the gpu.
+*/
+ if (bdev->pool.use_dma_alloc && cc_platform_has(CC_ATTR_MEM_ENCRYPT))
+ page_flags |= TTM_TT_FLAG_DECRYPTED;
bo->ttm = bdev->funcs->ttm_tt_create(bo, page_flags);
if (unlikely(bo->ttm == NULL))
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
index a4eff85b1f44..2b9d856ff388 100644
--- a/include/drm/ttm/ttm_tt.h
+++ b/include/drm/ttm/ttm_tt.h
@@ -79,6 +79,12 @@ struct ttm_tt {
* page_flags = TTM_TT_FLAG_EXTERNAL |
*TTM_TT_FLAG_EXTERNAL_MAPPABLE;
*
+* TTM_TT_FLAG_DECRYPTED: The mapped ttm pages should be marked as
+* not encrypted. The framework will try to match what the dma layer
+* is doing, but note that it is a little fragile because ttm page
+* fault handling abuses the DMA api a bit and dma_map_attrs can't be
+* used to assure pgprot always matches.
+*
* TTM_TT_FLAG_PRIV_POPULATED: TTM internal only. DO NOT USE. This is
* set by TTM after ttm_tt_populate() has successfully returned, and is
* then unset when TTM calls ttm_tt_unpopulate().
@@ -87,8 +93,9 @@ struct ttm_tt {
#define TTM_TT_FLAG_ZERO_ALLOC BIT(1)
#define TTM_TT_FLAG_EXTERNAL BIT(2)
#defi
Re: [PATCH] drm/i915: Don't set PIPE_CONTROL_FLUSH_L3 for aux inval
On Tue, Sep 26, 2023 at 04:24:01PM +0200, Nirmoy Das wrote:
> PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation
> so don't set that.
>
> Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before
> invalidation")
> Cc: Jonathan Cavitt
> Cc: Andi Shyti
> Cc: # v5.8+
> Cc: Andrzej Hajda
> Cc: Tvrtko Ursulin
> Cc: Matt Roper
> Cc: Tejas Upadhyay
> Cc: Lucas De Marchi
> Cc: Prathap Kumar Valsan
> Cc: Tapani Pälli
> Cc: Mark Janes
> Cc: Rodrigo Vivi
> Signed-off-by: Nirmoy Das
Acked-by: Matt Roper
> ---
> drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index 0143445dba83..ba4c2422b340 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -271,8 +271,17 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32
> mode)
> if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70))
> bit_group_0 |= PIPE_CONTROL_CCS_FLUSH;
>
> + /*
> + * L3 fabric flush is needed for AUX CCS invalidation
> + * which happens as part of pipe-control so we can
> + * ignore PIPE_CONTROL_FLUSH_L3. Also PIPE_CONTROL_FLUSH_L3
> + * deals with Protected Memory which is not needed for
> + * AUX CCS invalidation and lead to unwanted side effects.
> + */
> + if (mode & EMIT_FLUSH)
> + bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
> +
> bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH;
> - bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
> bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
> bit_group_1 |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
> /* Wa_1409600907:tgl,adl-p */
> --
> 2.41.0
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
Re: [PATCH 06/15] platform/x86/amd/pmf: Add support to get inputs from other subsystems
On Fri, 22 Sep 2023, Shyam Sundar S K wrote:
> PMF driver sends changing inputs from each subystem to TA for evaluating
> the conditions in the policy binary.
>
> Add initial support of plumbing in the PMF driver for Smart PC to get
> information from other subsystems in the kernel.
>
> Signed-off-by: Shyam Sundar S K
> ---
> drivers/platform/x86/amd/pmf/Makefile | 2 +-
> drivers/platform/x86/amd/pmf/pmf.h| 18
> drivers/platform/x86/amd/pmf/spc.c| 118 ++
> drivers/platform/x86/amd/pmf/tee-if.c | 3 +
> 4 files changed, 140 insertions(+), 1 deletion(-)
> create mode 100644 drivers/platform/x86/amd/pmf/spc.c
>
> diff --git a/drivers/platform/x86/amd/pmf/Makefile
> b/drivers/platform/x86/amd/pmf/Makefile
> index d2746ee7369f..6b26e48ce8ad 100644
> --- a/drivers/platform/x86/amd/pmf/Makefile
> +++ b/drivers/platform/x86/amd/pmf/Makefile
> @@ -7,4 +7,4 @@
> obj-$(CONFIG_AMD_PMF) += amd-pmf.o
> amd-pmf-objs := core.o acpi.o sps.o \
> auto-mode.o cnqf.o \
> - tee-if.o
> + tee-if.o spc.o
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index 81acf2a37366..e64b4d285624 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -146,6 +146,21 @@ struct smu_pmf_metrics {
> u16 infra_gfx_maxfreq; /* in MHz */
> u16 skin_temp; /* in centi-Celsius */
> u16 device_state;
> + u16 curtemp; /* in centi-Celsius */
> + u16 filter_alpha_value;
> + u16 avg_gfx_clkfrequency;
> + u16 avg_fclk_frequency;
> + u16 avg_gfx_activity;
> + u16 avg_socclk_frequency;
> + u16 avg_vclk_frequency;
> + u16 avg_vcn_activity;
> + u16 avg_dram_reads;
> + u16 avg_dram_writes;
> + u16 avg_socket_power;
> + u16 avg_core_power[2];
> + u16 avg_core_c0residency[16];
> + u16 spare1;
> + u32 metrics_counter;
> } __packed;
>
> enum amd_stt_skin_temp {
> @@ -592,4 +607,7 @@ extern const struct attribute_group
> cnqf_feature_attribute_group;
> int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
> int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
> +
> +/* Smart PC - TA interfaces */
> +void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct
> ta_pmf_enact_table *in);
> #endif /* PMF_H */
> diff --git a/drivers/platform/x86/amd/pmf/spc.c
> b/drivers/platform/x86/amd/pmf/spc.c
> new file mode 100644
> index ..08159cd5f853
> --- /dev/null
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD Platform Management Framework Driver - Smart PC Capabilities
> + *
> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Authors: Shyam Sundar S K
> + * Patil Rajesh Reddy
> + */
> +
> +#include
> +#include
> +#include "pmf.h"
> +
> +static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct
> ta_pmf_enact_table *in)
> +{
> + u16 max, avg = 0;
> + int i;
> +
> + memset(dev->buf, 0, sizeof(dev->m_table));
> + amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
> + memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
> +
> + in->ev_info.socket_power = dev->m_table.apu_power +
> dev->m_table.dgpu_power;
> + in->ev_info.skin_temperature = dev->m_table.skin_temp;
> +
> + /* get the avg C0 residency of all the cores */
> + for (i = 0; i < ARRAY_SIZE(dev->m_table.avg_core_c0residency); i++)
> + avg += dev->m_table.avg_core_c0residency[i];
Is this safe from overflow?
> +
> + /* get the max C0 residency of all the cores */
> + max = dev->m_table.avg_core_c0residency[0];
> + for (i = 1; i < ARRAY_SIZE(dev->m_table.avg_core_c0residency); i++) {
> + if (dev->m_table.avg_core_c0residency[i] > max)
> + max = dev->m_table.avg_core_c0residency[i];
> + }
> +
> + in->ev_info.avg_c0residency = avg /
> ARRAY_SIZE(dev->m_table.avg_core_c0residency);
> + in->ev_info.max_c0residency = max;
> + in->ev_info.gfx_busy = dev->m_table.avg_gfx_activity;
> +}
> +
> +static const char * const pmf_battery_supply_name[] = {
> + "BATT",
> + "BAT0",
> +};
> +
> +static int get_battery_prop(enum power_supply_property prop)
> +{
> + union power_supply_propval value;
> + struct power_supply *psy;
> + int i, ret = -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(pmf_battery_supply_name); i++) {
> + psy = power_supply_get_by_name(pmf_battery_supply_name[i]);
> + if (!psy)
> + continue;
> +
> + ret = power_supply_get_property(psy, prop, &value);
> + if (ret) {
> + power_supply_put(psy);
> + return ret;
> + }
> + }
> +
> + return value.intval;
> +}
> +
> +static int amd_pmf_get_battery_info(stru
Re: [PATCH 04/15] platform/x86/amd/pmf: Add support for PMF Policy Binary
On Fri, 22 Sep 2023, Shyam Sundar S K wrote:
> PMF Policy binary is a encrypted and signed binary that will be part
> of the BIOS. PMF driver via the ACPI interface checks the existence
> of Smart PC bit. If the advertised bit is found, PMF driver walks
> the acpi namespace to find out the policy binary size and the address
> which has to be passed to the TA during the TA init sequence.
>
> The policy binary is comprised of inputs (or the events) and outputs
> (or the actions). With the PMF ecosystem, OEMs generate the policy
> binary (or could be multiple binaries) that contains a supported set
> of inputs and outputs which could be specifically carved out for each
> usage segment (or for each user also) that could influence the system
> behavior either by enriching the user experience or/and boost/throttle
> power limits.
>
> Once the TA init command succeeds, the PMF driver sends the changing
> events in the current environment to the TA for a constant sampling
> frequency time (the event here could be a lid close or open) and
> if the policy binary has corresponding action built within it, the
> TA sends the action for it in the subsequent enact command.
>
> If the inputs sent to the TA has no output defined in the policy
> binary generated by OEMs, there will be no action to be performed
> by the PMF driver.
>
> Example policies:
>
> 1) if slider is performance ; set the SPL to 40W
> Here PMF driver registers with the platform profile interface and
> when the slider position is changed, PMF driver lets the TA know
> about this. TA sends back an action to update the Sustained
> Power Limit (SPL). PMF driver updates this limit via the PMFW mailbox.
>
> 2) if user_away ; then lock the system
> Here PMF driver hooks to the AMD SFH driver to know the user presence
> and send the inputs to TA and if the condition is met, the TA sends
> the action of locking the system. PMF driver generates a uevent and
> based on the udev rule in the userland the system gets locked with
> systemctl.
>
> The intent here is to provide the OEM's to make a policy to lock the
> system when the user is away ; but the userland can make a choice to
> ignore it.
>
> and so on.
>
> The OEMs will have an utility to create numerous such policies and
> the policies shall be reviewed by AMD before signing and encrypting
> them. Policies are shared between operating systems to have seemless user
> experience.
>
> Since all this action has to happen via the "amdtee" driver, currently
> there is no caller for it in the kernel which can load the amdtee driver.
> Without amdtee driver loading onto the system the "tee" calls shall fail
> from the PMF driver. Hence an explicit "request_module" has been added
> to address this.
>
> Signed-off-by: Shyam Sundar S K
> ---
> drivers/platform/x86/amd/pmf/Kconfig | 1 +
> drivers/platform/x86/amd/pmf/acpi.c | 37 +++
> drivers/platform/x86/amd/pmf/core.c | 12 +++
> drivers/platform/x86/amd/pmf/pmf.h| 132
> drivers/platform/x86/amd/pmf/tee-if.c | 141 +-
> 5 files changed, 321 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/Kconfig
> b/drivers/platform/x86/amd/pmf/Kconfig
> index 3064bc8ea167..437b78c6d1c5 100644
> --- a/drivers/platform/x86/amd/pmf/Kconfig
> +++ b/drivers/platform/x86/amd/pmf/Kconfig
> @@ -9,6 +9,7 @@ config AMD_PMF
> depends on POWER_SUPPLY
> depends on AMD_NB
> select ACPI_PLATFORM_PROFILE
> + depends on AMDTEE
> help
> This driver provides support for the AMD Platform Management
> Framework.
> The goal is to enhance end user experience by making AMD PCs smarter,
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c
> b/drivers/platform/x86/amd/pmf/acpi.c
> index 3fc5e4547d9f..d0512af2cd42 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -286,6 +286,43 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev)
> return 0;
> }
>
> +static acpi_status apmf_walk_resources(struct acpi_resource *res, void *data)
> +{
> + struct amd_pmf_dev *dev = data;
> +
> + switch (res->type) {
> + case ACPI_RESOURCE_TYPE_ADDRESS64:
> + dev->policy_addr = res->data.address64.address.minimum;
> + dev->policy_sz = res->data.address64.address.address_length;
> + break;
> + case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> + dev->policy_addr = res->data.fixed_memory32.address;
> + dev->policy_sz = res->data.fixed_memory32.address_length;
> + break;
> + }
> +
> + if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ ||
> dev->policy_sz == 0) {
> + pr_err("Incorrect Policy params, possibly a SBIOS bug\n");
> + return AE_ERROR;
> + }
> +
> + return AE_OK;
> +}
> +
> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev)
> +{
> + acpi_handle ahandle = ACPI_HANDLE(pmf_dev->
Re: [PATCH 05/15] platform/x86/amd/pmf: change debugfs init sequence
On Fri, 22 Sep 2023, Shyam Sundar S K wrote: > amd_pmf_dbgfs_register() needs to be called before amd_pmf_init_features(). Please answer to why? question too here. > Hence change the sequence. > > Reviewed-by: Mario Limonciello > Signed-off-by: Shyam Sundar S K > --- > drivers/platform/x86/amd/pmf/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/amd/pmf/core.c > b/drivers/platform/x86/amd/pmf/core.c > index 6f36c43e081e..dbfe7c1d6fc4 100644 > --- a/drivers/platform/x86/amd/pmf/core.c > +++ b/drivers/platform/x86/amd/pmf/core.c > @@ -427,9 +427,9 @@ static int amd_pmf_probe(struct platform_device *pdev) > > apmf_acpi_init(dev); > platform_set_drvdata(pdev, dev); > + amd_pmf_dbgfs_register(dev); > amd_pmf_init_features(dev); > apmf_install_handler(dev); > - amd_pmf_dbgfs_register(dev); > > dev_info(dev->dev, "registered PMF device successfully\n"); > > -- i.
Re: [PATCH 03/15] platform/x86/amd/pmf: Change signature of amd_pmf_set_dram_addr
On Fri, 22 Sep 2023, Shyam Sundar S K wrote:
Add () to the function name in the shortlog. "Change signature" is quite
vague, perhaps you could come up something more descriptive.
> Make amd_pmf_set_dram_addr() as non-static so that same function
> can be used across files.
This says nothing about the move of allocation.
> Reviewed-by: Mario Limonciello
> Signed-off-by: Shyam Sundar S K
> ---
> drivers/platform/x86/amd/pmf/core.c | 14 --
> drivers/platform/x86/amd/pmf/pmf.h | 1 +
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/core.c
> b/drivers/platform/x86/amd/pmf/core.c
> index 68f1389dda3e..5fb03ed614ff 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -251,26 +251,28 @@ static const struct pci_device_id pmf_pci_ids[] = {
> { }
> };
>
> -static void amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
> +int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
> {
> u64 phys_addr;
> u32 hi, low;
>
> + /* Get Metrics Table Address */
> + dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
> + if (!dev->buf)
> + return -ENOMEM;
> +
> phys_addr = virt_to_phys(dev->buf);
> hi = phys_addr >> 32;
> low = phys_addr & GENMASK(31, 0);
>
> amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
> amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
> +
> + return 0;
> }
>
> int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
> {
> - /* Get Metrics Table Address */
> - dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
> - if (!dev->buf)
> - return -ENOMEM;
> -
> INIT_DELAYED_WORK(&dev->work_buffer, amd_pmf_get_metrics);
>
> amd_pmf_set_dram_addr(dev);
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index a9333ff6c0a7..ea15ce547d24 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -421,6 +421,7 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev);
> int amd_pmf_get_power_source(void);
> int apmf_install_handler(struct amd_pmf_dev *pmf_dev);
> int apmf_os_power_slider_update(struct amd_pmf_dev *dev, u8 flag);
> +int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev);
>
> /* SPS Layer */
> int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);
>
Why are not amd_pmf_set_dram_addr() callers made to handle/pass on
errors???
--
i.
Re: [PATCH 02/15] platform/x86/amd/pmf: Add support PMF-TA interaction
On Fri, 22 Sep 2023, Shyam Sundar S K wrote:
> PMF TA (Trusted Application) loads via the TEE environment into the
> AMD ASP.
>
> PMF-TA supports two commands:
> 1) Init: Initialize the TA with the PMF Smart PC policy binary and
> start the policy engine. A policy is a combination of inputs and
> outputs, where;
> - the inputs are the changing dynamics of the system like the user
>behaviour, system heuristics etc.
> - the outputs, which are the actions to be set on the system which
>lead to better power management and enhanced user experience.
>
> PMF driver acts as a central manager in this case to supply the
> inputs required to the TA (either by getting the information from
> the other kernel subsystems or from userland)
>
> 2) Enact: Enact the output actions from the TA. The action could be
> applying a new thermal limit to boost/throttle the power limits or
> change system behavior.
>
> Reviewed-by: Mario Limonciello
> Signed-off-by: Shyam Sundar S K
> ---
> drivers/platform/x86/amd/pmf/pmf.h| 10 +++
> drivers/platform/x86/amd/pmf/tee-if.c | 97 ++-
> 2 files changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index 02460c2a31ea..a9333ff6c0a7 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -59,6 +59,9 @@
> #define ARG_NONE 0
> #define AVG_SAMPLE_SIZE 3
>
> +/* TA macros */
> +#define PMF_TA_IF_VERSION__MAJOR 1
I suppose double _ is not intentional?
> +
> /* AMD PMF BIOS interfaces */
> struct apmf_verify_interface {
> u16 size;
> @@ -184,6 +187,7 @@ struct amd_pmf_dev {
> struct tee_shm *fw_shm_pool;
> u32 session_id;
> void *shbuf;
> + struct delayed_work pb_work;
> bool smart_pc_enabled;
> };
>
> @@ -395,6 +399,12 @@ struct apmf_dyn_slider_output {
> struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
> } __packed;
>
> +/* cmd ids for TA communication */
> +enum ta_pmf_command {
> + TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE,
> + TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES
Add comma to the second line too.
Did you mean to have double _?
> +};
> +
> struct ta_pmf_shared_memory {
> int command_id;
> int resp_id;
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
> b/drivers/platform/x86/amd/pmf/tee-if.c
> index b48340edbf44..1fce04beacb3 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -13,9 +13,96 @@
> #include "pmf.h"
>
> #define MAX_TEE_PARAM4
> +
> +/* Policy binary actions sampling frequency (in ms) */
> +static int pb_actions_ms = 1000;
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +module_param(pb_actions_ms, int, 0644);
> +MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency
> (default = 1000ms)");
> +#endif
> +
> static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
> 0xb1, 0x2d, 0xc5, 0x29, 0xb1,
> 0x3d, 0x85, 0x43);
>
> +static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
> + struct tee_ioctl_invoke_arg *arg,
> + struct tee_param *param)
> +{
> + memset(arg, 0, sizeof(*arg));
> + memset(param, 0, MAX_TEE_PARAM * sizeof(*param));
> +
> + arg->func = cmd;
> + arg->session = dev->session_id;
> + arg->num_params = MAX_TEE_PARAM;
> +
> + /* Fill invoke cmd params */
> + param[0].u.memref.size = sizeof(struct ta_pmf_shared_memory);
> + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
> + param[0].u.memref.shm = dev->fw_shm_pool;
> + param[0].u.memref.shm_offs = 0;
> +}
> +
> +static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> +{
> + struct ta_pmf_shared_memory *ta_sm = NULL;
> + struct tee_param param[MAX_TEE_PARAM];
> + struct tee_ioctl_invoke_arg arg;
> + int ret = 0;
> +
> + if (!dev->tee_ctx)
> + return -ENODEV;
> +
> + ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
> + memset(ta_sm, 0, sizeof(struct ta_pmf_shared_memory));
> + ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES;
> + ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
> +
> + amd_pmf_prepare_args(dev,
> TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES, &arg, param);
> +
> + ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
> + if (ret < 0 || arg.ret != 0) {
> + dev_err(dev->dev, "%s failed TEE err: %x, ret:%x\n", __func__,
> arg.ret, ret);
No __func__s please.
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
> +{
> + struct ta_pmf_shared_memory *ta_sm = NULL;
> + struct tee_param param[MAX_TEE_PARAM];
> + struct tee_ioctl_invoke_arg arg;
> + int ret = 0;
> +
> + if (!dev->tee
Re: [PATCH 01/15] platform/x86/amd/pmf: Add PMF TEE interface
On Fri, 22 Sep 2023, Shyam Sundar S K wrote:
> AMD PMF driver loads the PMF TA (Trusted Application) into the AMD
> ASP's (AMD Security Processor) TEE (Trusted Execution Environment).
>
> PMF Trusted Application is a secured firmware placed under
> /lib/firmware/amdtee gets loaded only when the TEE environment is
> initialized. Add the initial code path to build these pipes.
>
> Reviewed-by: Mario Limonciello
> Signed-off-by: Shyam Sundar S K
> ---
> drivers/platform/x86/amd/pmf/Makefile | 3 +-
> drivers/platform/x86/amd/pmf/core.c | 11 ++-
> drivers/platform/x86/amd/pmf/pmf.h| 16
> drivers/platform/x86/amd/pmf/tee-if.c | 106 ++
> 4 files changed, 132 insertions(+), 4 deletions(-)
> create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c
>
> diff --git a/drivers/platform/x86/amd/pmf/Makefile
> b/drivers/platform/x86/amd/pmf/Makefile
> index fdededf54392..d2746ee7369f 100644
> --- a/drivers/platform/x86/amd/pmf/Makefile
> +++ b/drivers/platform/x86/amd/pmf/Makefile
> @@ -6,4 +6,5 @@
>
> obj-$(CONFIG_AMD_PMF) += amd-pmf.o
> amd-pmf-objs := core.o acpi.o sps.o \
> - auto-mode.o cnqf.o
> + auto-mode.o cnqf.o \
> + tee-if.o
> diff --git a/drivers/platform/x86/amd/pmf/core.c
> b/drivers/platform/x86/amd/pmf/core.c
> index 78ed3ee22555..68f1389dda3e 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -309,8 +309,11 @@ static void amd_pmf_init_features(struct amd_pmf_dev
> *dev)
> dev_dbg(dev->dev, "SPS enabled and Platform Profiles
> registered\n");
> }
>
> - /* Enable Auto Mode */
> - if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
> + if (amd_pmf_init_smart_pc(dev)) {
> + /* Enable Smart PC Solution builder */
> + dev_dbg(dev->dev, "Smart PC Solution Enabled\n");
> + } else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
> + /* Enable Auto Mode */
> amd_pmf_init_auto_mode(dev);
> dev_dbg(dev->dev, "Auto Mode Init done\n");
> } else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
> @@ -330,7 +333,9 @@ static void amd_pmf_deinit_features(struct amd_pmf_dev
> *dev)
> amd_pmf_deinit_sps(dev);
> }
>
> - if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
> + if (dev->smart_pc_enabled) {
> + amd_pmf_deinit_smart_pc(dev);
> + } else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
> amd_pmf_deinit_auto_mode(dev);
> } else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
> is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_DC))
> {
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index deba88e6e4c8..02460c2a31ea 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -179,6 +179,12 @@ struct amd_pmf_dev {
> bool cnqf_enabled;
> bool cnqf_supported;
> struct notifier_block pwr_src_notifier;
> + /* Smart PC solution builder */
> + struct tee_context *tee_ctx;
> + struct tee_shm *fw_shm_pool;
> + u32 session_id;
> + void *shbuf;
> + bool smart_pc_enabled;
> };
>
> struct apmf_sps_prop_granular {
> @@ -389,6 +395,13 @@ struct apmf_dyn_slider_output {
> struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
> } __packed;
>
> +struct ta_pmf_shared_memory {
> + int command_id;
> + int resp_id;
> + u32 pmf_result;
> + u32 if_version;
> +};
> +
> /* Core Layer */
> int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
> void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev);
> @@ -433,4 +446,7 @@ void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev);
> int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t
> time_lapsed_ms);
> extern const struct attribute_group cnqf_feature_attribute_group;
>
> +/* Smart PC builder Layer*/
> +int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
> +void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
> #endif /* PMF_H */
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
> b/drivers/platform/x86/amd/pmf/tee-if.c
> new file mode 100644
> index ..b48340edbf44
> --- /dev/null
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD Platform Management Framework Driver - TEE Interface
> + *
> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Author: Shyam Sundar S K
> + */
> +
> +#include
> +#include
> +#include "pmf.h"
> +
> +#define MAX_TEE_PARAM4
> +static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
> + 0xb1, 0x2d, 0xc5, 0x29, 0xb1,
> 0x3d, 0x85, 0x43);
> +
> +static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const
> void *data)
>
RE: [PATCH 4/6] drm/edid: use a temp variable for sads to drop one level of dereferences
> -Original Message-
> From: Nikula, Jani
> Sent: 26 September 2023 13:14
> To: Golani, Mitulkumar Ajitkumar ;
> dri-devel@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org
> Subject: RE: [PATCH 4/6] drm/edid: use a temp variable for sads to drop one
> level of dereferences
>
> On Mon, 25 Sep 2023, "Golani, Mitulkumar Ajitkumar"
> wrote:
> > Hi Jani,
> >
> > added comments in-line.
> >
> >> -Original Message-
> >> From: Nikula, Jani
> >> Sent: Thursday, September 7, 2023 2:58 PM
> >> To: dri-devel@lists.freedesktop.org
> >> Cc: intel-...@lists.freedesktop.org; Nikula, Jani
> >> ; Golani, Mitulkumar Ajitkumar
> >>
> >> Subject: [PATCH 4/6] drm/edid: use a temp variable for sads to drop
> >> one level of dereferences
> >>
> >> It's arguably easier on the eyes, and drops a set of parenthesis too.
> >
> > Please consider providing a bit more context in the commit message for
> better clarity.
> >
> >>
> >> Cc: Mitul Golani
> >> Signed-off-by: Jani Nikula
> >> ---
> >> drivers/gpu/drm/drm_edid.c | 16 +---
> >> 1 file changed, 9 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> index 2025970816c9..fcdc2c314cde 100644
> >> --- a/drivers/gpu/drm/drm_edid.c
> >> +++ b/drivers/gpu/drm/drm_edid.c
> >> @@ -5583,7 +5583,7 @@ static void drm_edid_to_eld(struct
> >> drm_connector *connector, }
> >>
> >> static int _drm_edid_to_sad(const struct drm_edid *drm_edid,
> >> - struct cea_sad **sads)
> >> + struct cea_sad **psads)
> >> {
> >> const struct cea_db *db;
> >> struct cea_db_iter iter;
> >> @@ -5592,19 +5592,21 @@ static int _drm_edid_to_sad(const struct
> >> drm_edid *drm_edid,
> >> cea_db_iter_edid_begin(drm_edid, &iter);
> >> cea_db_iter_for_each(db, &iter) {
> >> if (cea_db_tag(db) == CTA_DB_AUDIO) {
> >> + struct cea_sad *sads;
> >> int j;
> >>
> >> count = cea_db_payload_len(db) / 3; /* SAD is 3B */
> >> - *sads = kcalloc(count, sizeof(**sads), GFP_KERNEL);
> >> - if (!*sads)
> >> + sads = kcalloc(count, sizeof(*sads), GFP_KERNEL);
> >> + *psads = sads;
> >> + if (!sads)
> >> return -ENOMEM;
> >> for (j = 0; j < count; j++) {
> >> const u8 *sad = &db->data[j * 3];
> >>
> >> - (*sads)[j].format = (sad[0] & 0x78) >> 3;
> >> - (*sads)[j].channels = sad[0] & 0x7;
> >> - (*sads)[j].freq = sad[1] & 0x7F;
> >> - (*sads)[j].byte2 = sad[2];
> >> + sads[j].format = (sad[0] & 0x78) >> 3;
> >> + sads[j].channels = sad[0] & 0x7;
> >> + sads[j].freq = sad[1] & 0x7F;
> >> + sads[j].byte2 = sad[2];
> >
> > Thanks for the code update. I noticed the use of magic values in this
> > section, which can make the code less clear and harder to maintain.
> > Would it be possible to define constants or use descriptive names instead
> of these magic values?
>
> Yes, but that would be for a separate patch. The magic values aren't added
> here.
Sure. Other changes looks good to me.
Reviewed-by: Mitul Golani
>
> BR,
> Jani.
>
> >
> > Regards,
> > Mitul
> >> }
> >> break;
> >> }
> >> --
> >> 2.39.2
> >
>
> --
> Jani Nikula, Intel
Re: [PATCH] drm/i915: Don't set PIPE_CONTROL_FLUSH_L3 for aux inval
Hi Nirmoy,
On Tue, Sep 26, 2023 at 04:24:01PM +0200, Nirmoy Das wrote:
> PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation
> so don't set that.
>
> Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before
> invalidation")
> Cc: Jonathan Cavitt
> Cc: Andi Shyti
> Cc: # v5.8+
> Cc: Andrzej Hajda
> Cc: Tvrtko Ursulin
> Cc: Matt Roper
> Cc: Tejas Upadhyay
> Cc: Lucas De Marchi
> Cc: Prathap Kumar Valsan
> Cc: Tapani Pälli
> Cc: Mark Janes
> Cc: Rodrigo Vivi
> Signed-off-by: Nirmoy Das
looks better :)
Tapani, you mind giving this a test?
Andi
Re: [PATCH 3/2] accel/ivpu: Use local variable for debugfs root
On 9/25/2023 11:26 PM, Stanislaw Gruszka wrote: Use local variable for debugfs root, just to make further changes easier. Signed-off-by: Stanislaw Gruszka Reviewed-by: Jeffrey Hugo
Re: [PATCH v2 6/6] accel/ivpu: Use cached buffers for FW loading
On 9/26/2023 6:09 AM, Stanislaw Gruszka wrote:
From: Karol Wachowski
Create buffers with cache coherency on the CPU side (write-back) while
disabling snooping on the VPU side. These buffers require an explicit
cache flush after each CPU-side modification.
Configuring pages as write-combined may introduce significant delays,
potentially taking hundreds of milliseconds for 64 MB buffers.
Added internal DRM_IVPU_BO_NOSNOOP mask which disables snooping on the
VPU side. Allocate FW runtime memory buffer (64 MB) as cached with
snooping-disabled.
This fixes random long FW loading times and boot params memory
corruption on warmboot (due to missed wmb).
Fixes: 02d5b0aacd05 ("accel/ivpu: Implement firmware parsing and booting")
Signed-off-by: Karol Wachowski
Reviewed-by: Stanislaw Gruszka
Signed-off-by: Stanislaw Gruszka
Reviewed-by: Jeffrey Hugo
Re: [PATCH 5/6] accel/ivpu/40xx: Fix missing VPUIP interrupts
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote:
From: Karol Wachowski
Move sequence of masking and unmasking global interrupts from buttress
interrupt handler to generic one that handles both VPUIP and BTRS
interrupts.
Unmasking global interrupts will re-trigger MSI for any pending interrupts.
Lack of this sequence can randomly cause to miss any VPUIP interrupt that
comes after reading VPU_40XX_HOST_SS_ICB_STATUS_0 and before clearing
all active interrupt sources.
Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
Signed-off-by: Karol Wachowski
Reviewed-by: Stanislaw Gruszka
Signed-off-by: Stanislaw Gruszka
Reviewed-by: Jeffrey Hugo
Re: [PATCH 4/6] accel/ivpu/40xx: Disable frequency change interrupt
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote:
From: Karol Wachowski
Do not enable frequency change interrupt on 40xx as it might
lead to an interrupt storm in current design.
FREQ_CHANGE interrupt is triggered on D0I2 entry which will cause
KMD to check VPU interrupt sources by reading VPUIP registers.
Access to those registers will toggle necessary clocks and trigger
another FREQ_CHANGE interrupt possibly ending in an infinite loop.
FREQ_CHANGE interrupt has only debug purposes and can be permanently
disabled.
Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
Signed-off-by: Karol Wachowski
Reviewed-by: Stanislaw Gruszka
Signed-off-by: Stanislaw Gruszka
Reviewed-by: Jeffrey Hugo
Re: [PATCH 3/6] accel/ivpu/40xx: Ensure clock resource ownership Ack before Power-Up
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote:
From: Karol Wachowski
We need to wait for the CLOCK_RESOURCE_OWN_ACK bit to be set
after configuring the workpoint. This step ensures that the VPU
microcontroller clock is actively toggling and ready for operation.
Previously, we relied solely on the READY bit in the VPU_STATUS
register, which indicated the completion of the workpoint download.
However, this approach was insufficient, as the READY bit could be set
while the device was still running on a sideband clock until the PLL
locked. To guarantee that the PLL is locked and the device is running on
the main clock source, we now wait for the CLOCK_RESOURCE_OWN_ACK before
proceeding with the remainder of the power-up sequence.
Fixes: 79cdc56c4a54 ("accel/ivpu: Add initial support for VPU 4")
Signed-off-by: Karol Wachowski
Reviewed-by: Stanislaw Gruszka
Signed-off-by: Stanislaw Gruszka
Reviewed-by: Jeffrey Hugo
Re: [PATCH 2/6] accel/ivpu: Don't flood dmesg with VPU ready message
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote: From: Jacek Lawrynowicz Use ivpu_dbg() to print the VPU ready message so it doesn't pollute the dmesg. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Stanislaw Gruszka Signed-off-by: Stanislaw Gruszka Reviewed-by: Jeffrey Hugo
Re: [PATCH 1/6] accel/ivpu: Do not use wait event interruptible
On 9/25/2023 6:11 AM, Stanislaw Gruszka wrote:
If we receive signal when waiting for IPC message response in
ivpu_ipc_receive() we return error and continue to operate.
Then the driver can send another IPC messages and re-use occupied
slot of the message still processed by the firmware. This can result
in corrupting firmware memory and following FW crash with messages:
[ 3698.569719] intel_vpu :00:0b.0: [drm] ivpu_ipc_send_receive_internal():
IPC receive failed: type 0x1103, ret -512
[ 3698.569747] intel_vpu :00:0b.0: [drm] ivpu_jsm_unregister_db(): Failed
to unregister doorbell 3: -512
[ 3698.569756] intel_vpu :00:0b.0: [drm] ivpu_ipc_tx_prepare(): IPC message
vpu:0x8898 not released by firmware
[ 3698.569763] intel_vpu :00:0b.0: [drm] ivpu_ipc_tx_prepare(): JSM message
vpu:0x88980040 not released by firmware
[ 3698.570234] intel_vpu :00:0b.0: [drm] ivpu_ipc_send_receive_internal():
IPC receive failed: type 0x110e, ret -512
[ 3698.570318] intel_vpu :00:0b.0: [drm] *ERROR* ivpu_mmu_dump_event(): MMU
EVTQ: 0x10 (Translation fault) SSID: 0 SID: 3, e[2] , e[3] 0208, in
addr: 0x88988000, fetch addr: 0x0
To fix the issue don't use interruptible variant of wait event to
allow firmware to finish IPC processing.
Fixes: 5d7422cfb498 ("accel/ivpu: Add IPC driver and JSM messages")
Reviewed-by: Karol Wachowski
Signed-off-by: Stanislaw Gruszka
Reviewed-by: Jeffrey Hugo
Re: [PATCH 5/6] drm/i915: Add stable memory region names
On 26/09/2023 16:29, Iddamsetty, Aravind wrote:
On 22-09-2023 19:16, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
At the moment memory region names are a bit too varied and too
inconsistent to be used for ABI purposes, like for upcoming fdinfo
memory stats.
System memory can be either system or system-ttm. Local memory has the
instance number appended, others do not. Not only incosistent but thi
kind of implementation detail is uninteresting for intended users of
fdinfo memory stats.
Add a stable name always formed as $type$instance. Could have chosen a
different stable scheme, but I think any consistent and stable scheme
should do just fine.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/intel_memory_region.c | 19 +++
drivers/gpu/drm/i915/intel_memory_region.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c
b/drivers/gpu/drm/i915/intel_memory_region.c
index 3d1fdea9811d..60a03340bbd4 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -216,6 +216,22 @@ static int intel_memory_region_memtest(struct
intel_memory_region *mem,
return err;
}
+static const char *region_type_str(u16 type)
+{
+ switch (type) {
+ case INTEL_MEMORY_SYSTEM:
+ return "system";
+ case INTEL_MEMORY_LOCAL:
+ return "local";
+ case INTEL_MEMORY_STOLEN_LOCAL:
+ return "stolen-local";
+ case INTEL_MEMORY_STOLEN_SYSTEM:
+ return "stolen-system";
+ default:
+ return "unknown";
+ }
+}
+
struct intel_memory_region *
intel_memory_region_create(struct drm_i915_private *i915,
resource_size_t start,
@@ -244,6 +260,9 @@ intel_memory_region_create(struct drm_i915_private *i915,
mem->type = type;
mem->instance = instance;
+ snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
+region_type_str(type), instance);
+
mutex_init(&mem->objects.lock);
INIT_LIST_HEAD(&mem->objects.list);
diff --git a/drivers/gpu/drm/i915/intel_memory_region.h b/drivers/gpu/drm/i915/intel_memory_region.h
index 2953ed5c3248..9ba36454e51b 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.h
+++ b/drivers/gpu/drm/i915/intel_memory_region.h
@@ -80,6 +80,7 @@ struct intel_memory_region {
u16 instance;
enum intel_region_id id;
char name[16];
+ char uabi_name[16];
Just a thought instead of creating a new field, can't we derive this
with name and instance?
I'd rather not snprintf on every fdinfo read - for every pid and every
drm fd versus 2-3 strings kept around.
I did briefly wonder if mr->name could be dropped, that is renamed to
mr->uabi_name, but I guess there is some value to print the internal
name in some log messages, to leave a trace of what underlying
implementation is used. Although I am not too sure about the value of
that either since it is implied from the kernel version.
Then on top the usage in i915_gem_create/repr_name I could replace with
mr->uabi_name and simplify. If there is any value in printing the name
there, versus just uabi type:instance integers. Dunno. All I know is
fdinfo should have stable names and not confuse with implementation
details so I need something..
Regards,
Tvrtko
Re: [PATCH 5/6] drm/i915: Add stable memory region names
On 22-09-2023 19:16, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> At the moment memory region names are a bit too varied and too
> inconsistent to be used for ABI purposes, like for upcoming fdinfo
> memory stats.
>
> System memory can be either system or system-ttm. Local memory has the
> instance number appended, others do not. Not only incosistent but thi
> kind of implementation detail is uninteresting for intended users of
> fdinfo memory stats.
>
> Add a stable name always formed as $type$instance. Could have chosen a
> different stable scheme, but I think any consistent and stable scheme
> should do just fine.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> drivers/gpu/drm/i915/intel_memory_region.c | 19 +++
> drivers/gpu/drm/i915/intel_memory_region.h | 1 +
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_memory_region.c
> b/drivers/gpu/drm/i915/intel_memory_region.c
> index 3d1fdea9811d..60a03340bbd4 100644
> --- a/drivers/gpu/drm/i915/intel_memory_region.c
> +++ b/drivers/gpu/drm/i915/intel_memory_region.c
> @@ -216,6 +216,22 @@ static int intel_memory_region_memtest(struct
> intel_memory_region *mem,
> return err;
> }
>
> +static const char *region_type_str(u16 type)
> +{
> + switch (type) {
> + case INTEL_MEMORY_SYSTEM:
> + return "system";
> + case INTEL_MEMORY_LOCAL:
> + return "local";
> + case INTEL_MEMORY_STOLEN_LOCAL:
> + return "stolen-local";
> + case INTEL_MEMORY_STOLEN_SYSTEM:
> + return "stolen-system";
> + default:
> + return "unknown";
> + }
> +}
> +
> struct intel_memory_region *
> intel_memory_region_create(struct drm_i915_private *i915,
> resource_size_t start,
> @@ -244,6 +260,9 @@ intel_memory_region_create(struct drm_i915_private *i915,
> mem->type = type;
> mem->instance = instance;
>
> + snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
> + region_type_str(type), instance);
> +
> mutex_init(&mem->objects.lock);
> INIT_LIST_HEAD(&mem->objects.list);
>
> diff --git a/drivers/gpu/drm/i915/intel_memory_region.h
> b/drivers/gpu/drm/i915/intel_memory_region.h
> index 2953ed5c3248..9ba36454e51b 100644
> --- a/drivers/gpu/drm/i915/intel_memory_region.h
> +++ b/drivers/gpu/drm/i915/intel_memory_region.h
> @@ -80,6 +80,7 @@ struct intel_memory_region {
> u16 instance;
> enum intel_region_id id;
> char name[16];
> + char uabi_name[16];
Just a thought instead of creating a new field, can't we derive this
with name and instance?
Thanks,
Aravind.
> bool private; /* not for userspace */
>
> struct {
Re: [PATCH RFC v6 00/10] Support for Solid Fill Planes
On 2023-08-28 20:05, Jessica Zhang wrote: > Some drivers support hardware that have optimizations for solid fill > planes. This series aims to expose these capabilities to userspace as > some compositors have a solid fill flag (ex. SOLID_COLOR in the Android > hardware composer HAL) that can be set by apps like the Android Gears > app. > > In order to expose this capability to userspace, this series will: > > - Introduce solid_fill and pixel_source properties to allow userspace to > toggle between FB and solid fill sources > - Loosen NULL FB checks within the DRM atomic commit callstack to allow > for NULL FB when solid fill is enabled. > - Add NULL FB checks in methods where FB was previously assumed to be > non-NULL > - Have MSM DPU driver use drm_plane_state.solid_fill instead of > dpu_plane_state.color_fill > > Note: The solid fill planes feature depends on both the solid_fill *and* > pixel_source properties. > > To use this feature, userspace can set the solid_fill property to a blob > containing the appropriate version number and solid fill color (in > RGB323232 format) and and setting the pixel_source property to > DRM_PLANE_PIXEL_SOURCE_COLOR. This will disable memory fetch and the > resulting plane will display the color specified by the solid_fill blob. > > Currently, there's only one version of the solid_fill blob property. > However if other drivers want to support a similar feature, but require > more than just the solid fill color, they can extend this feature by > creating additional versions of the drm_solid_fill struct. > > This 2 property approach was chosen because passing in a special 1x1 FB > with the necessary color information would have unecessary overhead that > does not reflect the behavior of the solid fill feature. In addition, > assigning the solid fill blob to FB_ID would require loosening some core > drm_property checks that might cause unwanted side effects elsewhere. > I didn't have a detailed review of this patchset but at a high-level this change makes sense to me. Feel free to add my Acked-by: Harry Wentland to patches 1-5. Harry > --- > Changes in v6: > - Have _dpu_plane_color_fill() take in a single ABGR color instead > of having separate alpha and BGR color parameters (Dmitry) > - Drop plane->state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB check > in SetPlane ioctl (Dmitry) > - Add DRM_PLANE_PIXEL_SOURCE_NONE as a default pixel source (Sebastian) > - Dropped versioning from solid fill property blob (Dmitry) > - Use DRM_ENUM_NAME_FN (Dmitry) > - Use drm_atomic_replace_property_blob_from_id() (Dmitry) > - drm_atomic_check_fb -> drm_atomic_plane_check_fb (Dmitry) > - Group redundant NULL FB checks (Dmitry) > - Squashed drm_plane_needs_disable() implementation with > DRM_PLANE_PIXEL_SOURCE_NONE declaration (Sebastian) > - Add comment to support RGBA solid fill color in the future (Dmitry) > - Link to v5: > https://lore.kernel.org/r/20230728-solid-fill-v5-0-053dbefa9...@quicinc.com > > Changes in v5: > - Added support for PIXEL_SOURCE_NONE (Sebastian) > - Added WARN_ON() in drm_plane_has_visible_data() if pixel_source isn't > set (Dmitry) > - Added debugfs support for both properties (Dmitry) > - Corrected u32 to u8 conversion (Pekka) > - Moved drm_solid_fill_info struct and related documentation to > include/uapi (Pekka) > - Changed drm_solid_fill_info.version to __u32 for data alignment (Pekka) > - Added more detailed UAPI and kernel documentation (Pekka) > - Reordered patch series so that the pixel_source property is introduced > before solid_fill (Dmitry) > - Fixed inconsistent ABGR/RGBA format declaration (Pekka) > - Reset pixel_source to FB in drm_mode_setplane() (Dmitry) > - Rename supported_sources to extra_sources (Dmitry) > - Only destroy old solid_fill blob state if new state is valid (Pekka) > - Link to v4: > https://lore.kernel.org/r/20230404-solid-fill-v4-0-f4ec0caa7...@quicinc.com > > Changes in v4: > - Rebased onto latest kernel > - Reworded cover letter for clarity (Dmitry) > - Reworded commit messages for clarity > - Split existing changes into smaller commits > - Added pixel_source enum property (Dmitry, Pekka, Ville) > - Updated drm-kms comment docs with pixel_source and solid_fill > properties (Dmitry) > - Inlined drm_atomic_convert_solid_fill_info() (Dmitry) > - Passed in plane state alpha value to _dpu_plane_color_fill_pipe() > - Link to v3: > https://lore.kernel.org/r/20230104234036.636-1-quic_jessz...@quicinc.com > > Changes in v3: > - Fixed some logic errors in atomic checks (Dmitry) > - Introduced drm_plane_has_visible_data() and drm_atomic_check_fb() helper > methods (Dmitry) > - Fixed typo in drm_solid_fill struct documentation > - Created drm_plane_has_visible_data() helper and corrected CRTC and FB > NULL-check logic (Dmitry) > - Merged `if (fb)` blocks in drm_atomic_plane_check() and abstracted > them into helper method (Dmitry) > - Inverted `if (solid_fill_enabled) else if (fb)`
[PATCH v3] backlight: pwm_bl: Disable PWM on shutdown and suspend
Since commit 00e7e698bff1 ("backlight: pwm_bl: Configure pwm only once
per backlight toggle") calling pwm_backlight_power_off() doesn't disable
the PWM any more. However this is necessary to suspend because PWM
drivers usually refuse to suspend if they are still enabled.
Also adapt shutdown and remove callbacks to disable the PWM for similar
reasons.
Fixes: 00e7e698bff1 ("backlight: pwm_bl: Configure pwm only once per backlight
toggle")
Reported-by: Aisheng Dong
Tested-by: Aisheng Dong
Signed-off-by: Uwe Kleine-König
---
Hello,
On Tue, Sep 26, 2023 at 12:11:37PM +0100, Daniel Thompson wrote:
> Changes proposed look good (and the comment about badly designed boards
> going to HiZ state we super helpful).
I didn't mention Hi-Z and note that disabling a PWM can even result in
the hardware driving to the active level. (This can happen for example
for pwm-mxs and pwm-imx27.)
> Only thing from my is why there is no attempt to disable the PWM
> from the .remove_new() callback.
Good catch, good I didn't manage to send out a v3 for the email address
fix yet :-) So here comes a v3 with two improvments:
Changes since v2
(https://lore.kernel.org/dri-devel/20230926084612.2074692-1-u.kleine-koe...@pengutronix.de):
- Fix Aisheng Dong's email address
- also disable PWM in .remove and adapt commit log accordingly (Thanks
to Daniel Thompson for spotting that).
drivers/video/backlight/pwm_bl.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index a51fbab96368..390398ae07b9 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -629,6 +629,10 @@ static void pwm_backlight_remove(struct platform_device
*pdev)
backlight_device_unregister(bl);
pwm_backlight_power_off(pb);
+ pwm_get_state(pb->pwm, &state);
+ state.duty_cycle = 0;
+ state.enabled = false;
+ pwm_apply_state(pb->pwm, &state);
if (pb->exit)
pb->exit(&pdev->dev);
@@ -638,8 +642,13 @@ static void pwm_backlight_shutdown(struct platform_device
*pdev)
{
struct backlight_device *bl = platform_get_drvdata(pdev);
struct pwm_bl_data *pb = bl_get_data(bl);
+ struct pwm_state state;
pwm_backlight_power_off(pb);
+ pwm_get_state(pb->pwm, &state);
+ state.duty_cycle = 0;
+ state.enabled = false;
+ pwm_apply_state(pb->pwm, &state);
}
#ifdef CONFIG_PM_SLEEP
@@ -647,12 +656,24 @@ static int pwm_backlight_suspend(struct device *dev)
{
struct backlight_device *bl = dev_get_drvdata(dev);
struct pwm_bl_data *pb = bl_get_data(bl);
+ struct pwm_state state;
if (pb->notify)
pb->notify(pb->dev, 0);
pwm_backlight_power_off(pb);
+ /*
+* Note that disabling the PWM doesn't guarantee that the output stays
+* at its inactive state. However without the PWM disabled, the PWM
+* driver refuses to suspend. So disable here even though this might
+* enable the backlight on poorly designed boards.
+*/
+ pwm_get_state(pb->pwm, &state);
+ state.duty_cycle = 0;
+ state.enabled = false;
+ pwm_apply_state(pb->pwm, &state);
+
if (pb->notify_after)
pb->notify_after(pb->dev, 0);
base-commit: 8fff9184d1b5810dca5dd1a02726d4f844af88fc
--
2.40.1
Re: [GIT PULL] drm: renesas: shmobile: Atomic conversion + DT support (was: Re: [PATCH v4 00/41] drm: renesas: shmobile: Atomic conversion + DT support)
Hi Laurent, David, Daniel,
On Tue, Sep 19, 2023 at 5:24 PM Laurent Pinchart
wrote:
> On Tue, Sep 19, 2023 at 04:28:40PM +0200, Geert Uytterhoeven wrote:
> > The following changes since commit 0663e1da5ba8e6459e3555ac12c62741668c0d30:
> >
> > drm/dp_mst: Tune down error message during payload addition
> > (2023-09-18 16:38:21 +0300)
> >
> > are available in the Git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
> > tags/shmob-drm-atomic-dt-tag1
> >
> > for you to fetch changes up to bfea0fa9052aa8d235b24957eb84d9ff20cb87b7:
> >
> > drm: renesas: shmobile: Add DT support (2023-09-19 15:58:04 +0200)
> >
> >
> > drm: renesas: shmobile: Atomic conversion + DT support
> >
> > Currently, there are two drivers for the LCD controller on Renesas
> > SuperH-based and ARM-based SH-Mobile and R-Mobile SoCs:
> > 1. sh_mobile_lcdcfb, using the fbdev framework,
> > 2. shmob_drm, using the DRM framework.
> > However, only the former driver is used, as all platform support
> > integrates the former. None of these drivers support DT-based systems.
> >
> > Convert the SH-Mobile DRM driver to atomic modesetting, and add DT
> > support, complemented by the customary set of fixes and improvements.
> >
> > Link:
> > https://lore.kernel.org/r/cover.1694767208.git.geert+rene...@glider.be/
> >
> > This PR is based on today's drm-misc/for-linux-next, to avoid a
> > conflict with commit 775b0669e19f2e4a ("drm/shmobile: Convert to
> > platform remove callback returning void") in drm-misc/for-linux-next
Now drm-misc/for-linux-next (which is still at v6.5-rc2) has been
merged into drm/drm-next (which is at v6.6-rc2), do you want me to
rebase my branch to current drm/drm-next, or any other commit?
Thanks!
> > Thanks for pulling!
> >
> > Geert Uytterhoeven (36):
> > MAINTAINER: Create entry for Renesas SH-Mobile DRM drivers
>
> I'm technically listed as the maintainer for this driver until Geert
> takes over, so for this pull request,
>
> Acked-by: Laurent Pinchart
>
> And after that, shmobile won't need my ack to merge further changes :-)
>
> This is very nice work Geert. I'm looking forward to dropping the
> sh_mobile_lcdcfb driver.
Thank you!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions
Hi Am 26.09.23 um 16:15 schrieb Carlos Eduardo Gallo Filho: There's duplicated functions on drm that do the same job of calculating the size of planes from a drm_format_info and the size of its first plane. So this patchset throw away the more specific version intended to be used from a given framebuffer and make the generic version way more portable against the drivers. For both patches Reviewed-by: Thomas Zimmermann Best regards Thomas Thanks, Carlos --- v2: - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers". Carlos Eduardo Gallo Filho (2): drm: Remove plane hsub/vsub alignment requirement for core helpers drm: Replace drm_framebuffer plane size functions with its equivalents drivers/gpu/drm/drm_framebuffer.c | 64 ++--- drivers/gpu/drm/i915/display/intel_fb.c | 2 +- include/drm/drm_fourcc.h| 5 +- include/drm/drm_framebuffer.h | 5 -- 4 files changed, 8 insertions(+), 68 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH] drm/i915/mtl: Skip MCR ops for ring fault register
On Tue, Sep 26, 2023 at 04:18:42PM +0200, Nirmoy Das wrote:
> On MTL GEN12_RING_FAULT_REG is not replicated so don't
> do mcr based operation for this register.
>
> Signed-off-by: Nirmoy Das
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 14 +-
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 +
> drivers/gpu/drm/i915/i915_gpu_error.c | 11 ++-
> 3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 93062c35e072..d4de692e8be1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -262,10 +262,22 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
> I915_MASTER_ERROR_INTERRUPT);
> }
>
> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> + /*
> + * for media tile this ring fault register is not replicated
> + * so skip doing mcr ops on it.
> + */
> + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50) &&
This should be checking the media version rather than the graphics
version. I.e., "MEDIA_VER(i915) > 13" since it's possible future
versions of the media IP may change the behavior (independently of the
graphics IP versions).
Matt
> + gt->type == GT_MEDIA) {
> + intel_uncore_rmw(uncore, XELPMP_RING_FAULT_REG,
> + RING_FAULT_VALID, 0);
> + intel_uncore_posting_read(uncore,
> + XELPMP_RING_FAULT_REG);
> +
> + } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> intel_gt_mcr_multicast_rmw(gt, XEHP_RING_FAULT_REG,
> RING_FAULT_VALID, 0);
> intel_gt_mcr_read_any(gt, XEHP_RING_FAULT_REG);
> +
> } else if (GRAPHICS_VER(i915) >= 12) {
> intel_uncore_rmw(uncore, GEN12_RING_FAULT_REG,
> RING_FAULT_VALID, 0);
> intel_uncore_posting_read(uncore, GEN12_RING_FAULT_REG);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index cca4bac8f8b0..eecd0a87a647 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1084,6 +1084,7 @@
>
> #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> #define XEHP_RING_FAULT_REG MCR_REG(0xcec4)
> +#define XELPMP_RING_FAULT_REG_MMIO(0xcec4)
> #define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
> #define RING_FAULT_GTTSEL_MASK (1 << 11)
> #define RING_FAULT_SRCID(x)(((x) >> 3) & 0xff)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index f4ebcfb70289..83f1a729da8b 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1234,7 +1234,16 @@ static void engine_record_registers(struct
> intel_engine_coredump *ee)
> if (GRAPHICS_VER(i915) >= 6) {
> ee->rc_psmi = ENGINE_READ(engine, RING_PSMI_CTL);
>
> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
> + /*
> + * for media tile this ring fault register is not replicated
> + * so skip doing mcr ops on it.
> + */
> + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50) &&
> + engine->gt->type == GT_MEDIA)
> + ee->fault_reg = intel_uncore_read(engine->uncore,
> +
> XELPMP_RING_FAULT_REG);
> +
> + else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
> ee->fault_reg = intel_gt_mcr_read_any(engine->gt,
>
> XEHP_RING_FAULT_REG);
> else if (GRAPHICS_VER(i915) >= 12)
> --
> 2.41.0
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
Re: [RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions
On 26/09/2023 11:15, Carlos Eduardo Gallo Filho wrote: There's duplicated functions on drm that do the same job of calculating the size of planes from a drm_format_info and the size of its first plane. So this patchset throw away the more specific version intended to be used from a given framebuffer and make the generic version way more portable against the drivers. Thanks, Carlos Hey, thanks for your patch. Do you mind testing on drm/ci and sending here the link of the pipeline with the test? It would be awesome to get your feedback on the CI See instructions on Documentation/gpu/automated_testing.rst In short you just need an account on gitlab.freedesktop.org, access https://gitlab.freedesktop.org/janedoe/linux/-/settings/ci_cd), change the CI/CD configuration file from .gitlab-ci.yml to drivers/gpu/drm/ci/gitlab-ci.yml, now you can execute tests going to pipelines (the first one you need to create a new pipeline by hand). Let me know if you have any questions, I'm koike on irc. Thanks Helen --- v2: - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers". Carlos Eduardo Gallo Filho (2): drm: Remove plane hsub/vsub alignment requirement for core helpers drm: Replace drm_framebuffer plane size functions with its equivalents drivers/gpu/drm/drm_framebuffer.c | 64 ++--- drivers/gpu/drm/i915/display/intel_fb.c | 2 +- include/drm/drm_fourcc.h| 5 +- include/drm/drm_framebuffer.h | 5 -- 4 files changed, 8 insertions(+), 68 deletions(-)
Re: Blank screen on boot of Linux 6.5 and later on Lenovo ThinkPad L570
Hi, all,
On Tue, Sep 26, 2023 at 7:15 PM Linux regression tracking (Thorsten
Leemhuis) wrote:
>
> [CCing the regression list, as it should be in the loop for regressions:
> https://docs.kernel.org/admin-guide/reporting-regressions.html]
>
> Hi, Thorsten here, the Linux kernel's regression tracker.
>
> On 13.09.23 14:02, Jaak Ristioja wrote:
> >
> > Upgrading to Linux 6.5 on a Lenovo ThinkPad L570 (Integrated Intel HD
> > Graphics 620 (rev 02), Intel(R) Core(TM) i7-7500U) results in a blank
> > screen after boot until the display manager starts... if it does start
> > at all. Using the nomodeset kernel parameter seems to be a workaround.
> >
> > I've bisected this to commit 60aebc9559492cea6a9625f514a8041717e3a2e4
> > ("drivers/firmware: Move sysfb_init() from device_initcall to
> > subsys_initcall_sync").
>
> Hmmm, no reaction since it was posted a while ago, unless I'm missing
> something.
>
> Huacai Chen, did you maybe miss this report? The problem is apparently
> caused by a commit of yours (that Javier applied), you hence should look
> into this.
I'm sorry but it looks very strange, could you please share your config file?
Huacai
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> > git bisect start
> > # status: waiting for both good and bad commits
> > # good: [6995e2de6891c724bfeb2db33d7b87775f913ad1] Linux 6.4
> > git bisect good 6995e2de6891c724bfeb2db33d7b87775f913ad1
> > # status: waiting for bad commit, 1 good commit known
> > # bad: [2dde18cd1d8fac735875f2e4987f11817cc0bc2c] Linux 6.5
> > git bisect bad 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> > # bad: [b775d6c5859affe00527cbe74263de05cfe6b9f9] Merge tag 'mips_6.5'
> > of git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux
> > git bisect bad b775d6c5859affe00527cbe74263de05cfe6b9f9
> > # good: [3a8a670eeeaa40d87bd38a587438952741980c18] Merge tag
> > 'net-next-6.5' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
> > git bisect good 3a8a670eeeaa40d87bd38a587438952741980c18
> > # bad: [188d3f80fc6d8451ab5e570becd6a7b2d3033023] drm/amdgpu: vcn_4_0
> > set instance 0 init sched score to 1
> > git bisect bad 188d3f80fc6d8451ab5e570becd6a7b2d3033023
> > # good: [12fb1ad70d65edc3405884792d044fa79df7244f] drm/amdkfd: update
> > process interrupt handling for debug events
> > git bisect good 12fb1ad70d65edc3405884792d044fa79df7244f
> > # bad: [9cc31938d4586f72eb8e0235ad9d9eb22496fcee] i915/perf: Drop the
> > aging_tail logic in perf OA
> > git bisect bad 9cc31938d4586f72eb8e0235ad9d9eb22496fcee
> > # bad: [51d86ee5e07ccef85af04ee9850b0baa107999b6] drm/msm: Switch to
> > fdinfo helper
> > git bisect bad 51d86ee5e07ccef85af04ee9850b0baa107999b6
> > # good: [bfdede3a58ea970333d77a05144a7bcec13cf515] drm/rockchip: cdn-dp:
> > call drm_connector_update_edid_property() unconditionally
> > git bisect good bfdede3a58ea970333d77a05144a7bcec13cf515
> > # good: [123ee07ba5b7123e0ce0e0f9d64938026c16a2ce] drm: sun4i_tcon: use
> > devm_clk_get_enabled in `sun4i_tcon_init_clocks`
> > git bisect good 123ee07ba5b7123e0ce0e0f9d64938026c16a2ce
> > # bad: [20d54e48d9c705091a025afff5839da2ea606f6b] fbdev: Rename
> > fb_mem*() helpers
> > git bisect bad 20d54e48d9c705091a025afff5839da2ea606f6b
> > # bad: [728cb3f061e2b3a002fd76d91c2449b1497b6640] gpu: drm: bridge: No
> > need to set device_driver owner
> > git bisect bad 728cb3f061e2b3a002fd76d91c2449b1497b6640
> > # bad: [0f1cb4d777281ca3360dbc8959befc488e0c327e] drm/ssd130x: Fix
> > include guard name
> > git bisect bad 0f1cb4d777281ca3360dbc8959befc488e0c327e
> > # good: [0bd5bd65cd2e4d1335ea6c17cd2c8664decbc630] dt-bindings: display:
> > simple: Add BOE EV121WXM-N10-1850 panel
> > git bisect good 0bd5bd65cd2e4d1335ea6c17cd2c8664decbc630
> > # bad: [60aebc9559492cea6a9625f514a8041717e3a2e4] drivers/firmware: Move
> > sysfb_init() from device_initcall to subsys_initcall_sync
> > git bisect bad 60aebc9559492cea6a9625f514a8041717e3a2e4
> > # good: [8bb7c7bca5b70f3cd22d95b4d36029295c4274f6] drm/panel:
> > panel-simple: Add BOE EV121WXM-N10-1850 panel support
> > git bisect good 8bb7c7bca5b70f3cd22d95b4d36029295c4274f6
> > # first bad commit: [60aebc9559492cea6a9625f514a8041717e3a2e4]
> > drivers/firmware: Move sysfb_init() from device_initcall to
> > subsys_initcall_sync
[PATCH] drm/i915: Don't set PIPE_CONTROL_FLUSH_L3 for aux inval
PIPE_CONTROL_FLUSH_L3 is not needed for aux invalidation
so don't set that.
Fixes: 78a6ccd65fa3 ("drm/i915/gt: Ensure memory quiesced before invalidation")
Cc: Jonathan Cavitt
Cc: Andi Shyti
Cc: # v5.8+
Cc: Andrzej Hajda
Cc: Tvrtko Ursulin
Cc: Matt Roper
Cc: Tejas Upadhyay
Cc: Lucas De Marchi
Cc: Prathap Kumar Valsan
Cc: Tapani Pälli
Cc: Mark Janes
Cc: Rodrigo Vivi
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 0143445dba83..ba4c2422b340 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -271,8 +271,17 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70))
bit_group_0 |= PIPE_CONTROL_CCS_FLUSH;
+ /*
+* L3 fabric flush is needed for AUX CCS invalidation
+* which happens as part of pipe-control so we can
+* ignore PIPE_CONTROL_FLUSH_L3. Also PIPE_CONTROL_FLUSH_L3
+* deals with Protected Memory which is not needed for
+* AUX CCS invalidation and lead to unwanted side effects.
+*/
+ if (mode & EMIT_FLUSH)
+ bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
+
bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH;
- bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
bit_group_1 |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
/* Wa_1409600907:tgl,adl-p */
--
2.41.0
[PATCH] drm/i915/mtl: Skip MCR ops for ring fault register
On MTL GEN12_RING_FAULT_REG is not replicated so don't
do mcr based operation for this register.
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/gt/intel_gt.c | 14 +-
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 +
drivers/gpu/drm/i915/i915_gpu_error.c | 11 ++-
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 93062c35e072..d4de692e8be1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -262,10 +262,22 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
I915_MASTER_ERROR_INTERRUPT);
}
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
+ /*
+* for media tile this ring fault register is not replicated
+* so skip doing mcr ops on it.
+*/
+ if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50) &&
+ gt->type == GT_MEDIA) {
+ intel_uncore_rmw(uncore, XELPMP_RING_FAULT_REG,
+RING_FAULT_VALID, 0);
+ intel_uncore_posting_read(uncore,
+ XELPMP_RING_FAULT_REG);
+
+ } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
intel_gt_mcr_multicast_rmw(gt, XEHP_RING_FAULT_REG,
RING_FAULT_VALID, 0);
intel_gt_mcr_read_any(gt, XEHP_RING_FAULT_REG);
+
} else if (GRAPHICS_VER(i915) >= 12) {
intel_uncore_rmw(uncore, GEN12_RING_FAULT_REG,
RING_FAULT_VALID, 0);
intel_uncore_posting_read(uncore, GEN12_RING_FAULT_REG);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index cca4bac8f8b0..eecd0a87a647 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1084,6 +1084,7 @@
#define GEN12_RING_FAULT_REG _MMIO(0xcec4)
#define XEHP_RING_FAULT_REGMCR_REG(0xcec4)
+#define XELPMP_RING_FAULT_REG _MMIO(0xcec4)
#define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
#define RING_FAULT_GTTSEL_MASK (1 << 11)
#define RING_FAULT_SRCID(x) (((x) >> 3) & 0xff)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index f4ebcfb70289..83f1a729da8b 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1234,7 +1234,16 @@ static void engine_record_registers(struct
intel_engine_coredump *ee)
if (GRAPHICS_VER(i915) >= 6) {
ee->rc_psmi = ENGINE_READ(engine, RING_PSMI_CTL);
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
+ /*
+* for media tile this ring fault register is not replicated
+* so skip doing mcr ops on it.
+*/
+ if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50) &&
+ engine->gt->type == GT_MEDIA)
+ ee->fault_reg = intel_uncore_read(engine->uncore,
+
XELPMP_RING_FAULT_REG);
+
+ else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
ee->fault_reg = intel_gt_mcr_read_any(engine->gt,
XEHP_RING_FAULT_REG);
else if (GRAPHICS_VER(i915) >= 12)
--
2.41.0
[PATCH v2 2/2] drm: Replace drm_framebuffer plane size functions with its equivalents
The functions drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} do exactly the same job of its
equivalents drm_format_info_plane_{width,height} from drm_fourcc.
The only reason to have these functions on drm_framebuffer
would be if they would added a abstraction layer to call it just
passing a drm_framebuffer pointer and the desired plane index,
which is not the case, where these functions actually implements
just part of it. In the actual implementation, every call to both
drm_framebuffer_plane_{width,height} and fb_plane_{width,height} should
pass some drm_framebuffer attribute, which is the same as calling the
drm_format_info_plane_{width,height} functions.
The drm_format_info_pane_{width,height} functions are much more
consistent in both its implementation and its location on code. The
kind of calculation that they do is intrinsically derivated from the
drm_format_info struct and has not to do with drm_framebuffer, except
by the potential motivation described above, which is still not a good
justification to have drm_framebuffer functions to calculate it.
So, replace each drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} call to drm_format_info_plane_{width,height}
and remove them.
Signed-off-by: Carlos Eduardo Gallo Filho
---
drivers/gpu/drm/drm_framebuffer.c | 64 ++---
drivers/gpu/drm/i915/display/intel_fb.c | 2 +-
include/drm/drm_framebuffer.h | 5 --
3 files changed, 5 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/drm_framebuffer.c
b/drivers/gpu/drm/drm_framebuffer.c
index ba51deb6d042..d3ba0698b84b 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -151,24 +151,6 @@ int drm_mode_addfb_ioctl(struct drm_device *dev,
return drm_mode_addfb(dev, data, file_priv);
}
-static int fb_plane_width(int width,
- const struct drm_format_info *format, int plane)
-{
- if (plane == 0)
- return width;
-
- return DIV_ROUND_UP(width, format->hsub);
-}
-
-static int fb_plane_height(int height,
- const struct drm_format_info *format, int plane)
-{
- if (plane == 0)
- return height;
-
- return DIV_ROUND_UP(height, format->vsub);
-}
-
static int framebuffer_check(struct drm_device *dev,
const struct drm_mode_fb_cmd2 *r)
{
@@ -196,8 +178,8 @@ static int framebuffer_check(struct drm_device *dev,
info = drm_get_format_info(dev, r);
for (i = 0; i < info->num_planes; i++) {
- unsigned int width = fb_plane_width(r->width, info, i);
- unsigned int height = fb_plane_height(r->height, info, i);
+ unsigned int width = drm_format_info_plane_width(info,
r->width, i);
+ unsigned int height = drm_format_info_plane_height(info,
r->height, i);
unsigned int block_size = info->char_per_block[i];
u64 min_pitch = drm_format_info_min_pitch(info, i, width);
@@ -1136,44 +1118,6 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
}
EXPORT_SYMBOL(drm_framebuffer_remove);
-/**
- * drm_framebuffer_plane_width - width of the plane given the first plane
- * @width: width of the first plane
- * @fb: the framebuffer
- * @plane: plane index
- *
- * Returns:
- * The width of @plane, given that the width of the first plane is @width.
- */
-int drm_framebuffer_plane_width(int width,
- const struct drm_framebuffer *fb, int plane)
-{
- if (plane >= fb->format->num_planes)
- return 0;
-
- return fb_plane_width(width, fb->format, plane);
-}
-EXPORT_SYMBOL(drm_framebuffer_plane_width);
-
-/**
- * drm_framebuffer_plane_height - height of the plane given the first plane
- * @height: height of the first plane
- * @fb: the framebuffer
- * @plane: plane index
- *
- * Returns:
- * The height of @plane, given that the height of the first plane is @height.
- */
-int drm_framebuffer_plane_height(int height,
-const struct drm_framebuffer *fb, int plane)
-{
- if (plane >= fb->format->num_planes)
- return 0;
-
- return fb_plane_height(height, fb->format, plane);
-}
-EXPORT_SYMBOL(drm_framebuffer_plane_height);
-
void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
const struct drm_framebuffer *fb)
{
@@ -1189,8 +1133,8 @@ void drm_framebuffer_print_info(struct drm_printer *p,
unsigned int indent,
for (i = 0; i < fb->format->num_planes; i++) {
drm_printf_indent(p, indent + 1, "size[%u]=%dx%d\n", i,
- drm_framebuffer_plane_width(fb->width, fb, i),
- drm_framebuffer_plane_height(fb->height, fb,
i));
+ drm_format_info_plane_width(fb->format,
fb->width, i),
+ drm_forma
[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers
The drm_format_info_plane_{height,width} functions was implemented using
regular division for the plane size calculation, which cause issues [1][2]
when used on contexts where the dimensions are misaligned with relation
to the subsampling factors. So, replace the regular division by the
DIV_ROUND_UP macro.
This allows these functions to be used in more drivers, making further
work to bring more core presence on them possible.
[1]
http://patchwork.freedesktop.org/patch/msgid/20170321181218.10042-3-ville.syrj...@linux.intel.com
[2]
https://patchwork.freedesktop.org/patch/msgid/20211026225105.2783797-2-imre.d...@intel.com
Signed-off-by: Carlos Eduardo Gallo Filho
---
include/drm/drm_fourcc.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 532ae78ca747..ccf91daa4307 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -22,6 +22,7 @@
#ifndef __DRM_FOURCC_H__
#define __DRM_FOURCC_H__
+#include
#include
#include
@@ -279,7 +280,7 @@ int drm_format_info_plane_width(const struct
drm_format_info *info, int width,
if (plane == 0)
return width;
- return width / info->hsub;
+ return DIV_ROUND_UP(width, info->hsub);
}
/**
@@ -301,7 +302,7 @@ int drm_format_info_plane_height(const struct
drm_format_info *info, int height,
if (plane == 0)
return height;
- return height / info->vsub;
+ return DIV_ROUND_UP(height, info->vsub);
}
const struct drm_format_info *__drm_format_info(u32 format);
--
2.41.0
[RESEND PATCH v2 0/2] drm: Refactor plane size calculation by core helper functions
There's duplicated functions on drm that do the same job of calculating the size of planes from a drm_format_info and the size of its first plane. So this patchset throw away the more specific version intended to be used from a given framebuffer and make the generic version way more portable against the drivers. Thanks, Carlos --- v2: - New patch "[PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers". Carlos Eduardo Gallo Filho (2): drm: Remove plane hsub/vsub alignment requirement for core helpers drm: Replace drm_framebuffer plane size functions with its equivalents drivers/gpu/drm/drm_framebuffer.c | 64 ++--- drivers/gpu/drm/i915/display/intel_fb.c | 2 +- include/drm/drm_fourcc.h| 5 +- include/drm/drm_framebuffer.h | 5 -- 4 files changed, 8 insertions(+), 68 deletions(-) -- 2.41.0
Re: [PATCH v2 00/15] drm: Add a driver for FW-based Mali GPUs
On Wed, Aug 9, 2023 at 10:53 AM Boris Brezillon wrote: > > Hello, > > This is the second version of the kernel driver meant to support new Mali > GPUs which are delegating the scheduling to a firmware. [...] > > I tried to Cc anyone that was involved in any development of the code > I picked from panfrost, so they can acknowledge the GPL2 -> MIT+GPL2 > change. If I missed someone, please let me know. Regarding the relicensing, Linaro agrees to the relicensing of the parts they hold copyright on. Acked-by: Grant Likely -- Grant Likely CTO of Linaro, Ltd. grant.lik...@linaro.org
Re: [PATCH] drm/panel: Move AUX B116XW03 out of panel-edp back to panel-simple
Hi,
On Tue, Sep 26, 2023 at 1:06 AM AngeloGioacchino Del Regno
wrote:
>
> Il 26/09/23 00:00, Douglas Anderson ha scritto:
> > In commit 5f04e7ce392d ("drm/panel-edp: Split eDP panels out of
> > panel-simple") I moved a pile of panels out of panel-simple driver
> > into the newly created panel-edp driver. One of those panels, however,
> > shouldn't have been moved.
> >
> > As is clear from commit e35e305eff0f ("drm/panel: simple: Add AUO
> > B116XW03 panel support"), AUX B116XW03 is an LVDS panel. It's used in
> > exynos5250-snow and exynos5420-peach-pit where it's clear that the
> > panel is hooked up with LVDS. Furthermore, searching for datasheets I
> > found one that makes it clear that this panel is LVDS.
> >
> > As far as I can tell, I got confused because in commit 88d3457ceb82
> > ("drm/panel: auo,b116xw03: fix flash backlight when power on") Jitao
> > Shi added "DRM_MODE_CONNECTOR_eDP". That seems wrong. Looking at the
> > downstream ChromeOS trees, it seems like some Mediatek boards are
> > using a panel that they call "auo,b116xw03" that's an eDP panel. The
> > best I can guess is that they actually have a different panel that has
> > similar timing. If so then the proper panel should be used or they
> > should switch to the generic "edp-panel" compatible.
> >
> > When moving this back to panel-edp, I wasn't sure what to use for
> > .bus_flags and .bus_format and whether to add the extra "enable" delay
> > from commit 88d3457ceb82 ("drm/panel: auo,b116xw03: fix flash
> > backlight when power on"). I've added formats/flags/delays based on my
> > (inexpert) analysis of the datasheet. These are untested.
> >
> > NOTE: if/when this is backported to stable, we might run into some
> > trouble. Specifically, before 474c162878ba ("arm64: dts: mt8183:
> > jacuzzi: Move panel under aux-bus") this panel was used by
> > "mt8183-kukui-jacuzzi", which assumed it was an eDP panel. I don't
> > know what to suggest for that other than someone making up a bogus
> > panel for jacuzzi that's just for the stable channel.
> >
> > Fixes: 88d3457ceb82 ("drm/panel: auo,b116xw03: fix flash backlight when
> > power on")
> > Fixes: 5f04e7ce392d ("drm/panel-edp: Split eDP panels out of panel-simple")
> > Signed-off-by: Douglas Anderson
> > ---
> > I haven't had a snow or peach-pit hooked up for debugging / testing
> > for years. I presume that they must be broken and hope that this fixes
> > them.
>
> We could avoid backport breakages by avoiding to backport this to any kernel
> that doesn't contain commit 474c162878ba ("arm64: dts: mt8183: jacuzzi: Move
> panel under aux-bus")... because creating a dummy panel to get two wrongs
> right is definitely not ok.
Sure, except that leaves us with ... a breakage. :-P
Although I haven't tested it, I have a hard time believing that
exynos5250-snow and exynos5420-peach-pit will work properly with the
panel defined as an eDP panel. That means that they will be broken. If
someone cared to get those fixed in a stable backport then we'd be
stuck deciding who to break. If you have any brilliant ideas then I'm
all ears.
...then again, I presume this has been broken since commit
88d3457ceb82 ("drm/panel: auo,b116xw03: fix flash backlight when power
on"). That was a little over 3 years ago. Maybe I'm wrong and somehow
things still limp along and sorta work even though the panel is
defined incorrectly?
-Doug
Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
Hi Christian,
On 9/26/2023 6:47 PM, Christian König wrote:
> Am 26.09.23 um 14:56 schrieb Hans de Goede:
>> Hi,
>>
>> On 9/26/23 13:24, Shyam Sundar S K wrote:
>>> Hi Hans,
>>>
>>> On 9/26/2023 4:05 PM, Hans de Goede wrote:
Hi,
On 9/22/23 19:50, Shyam Sundar S K wrote:
> For the Smart PC Solution to fully work, it has to enact to the
> actions
> coming from TA. Add the initial code path for set interface to
> AMDGPU.
>
> Co-developed-by: Mario Limonciello
> Signed-off-by: Mario Limonciello
> Signed-off-by: Shyam Sundar S K
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +
> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
> drivers/platform/x86/amd/pmf/tee-if.c | 19 +--
> include/linux/amd-pmf-io.h | 1 +
> 4 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> index 232d11833ddc..5c567bff0548 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct
> amd_gpu_pmf_data *pmf)
> return 0;
> }
> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
> +
> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
> +{
> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
> + struct backlight_device *bd;
> +
> + if (!(adev->flags & AMD_IS_APU)) {
> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
> + return -ENODEV;
> + }
> +
> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
> + if (!bd)
> + return -ENODEV;
This assumes that the backlight is always controller by the amdgpu's
native backlight driver, but it might e.g. also be handled by
eacpi-video or by nvidia_wmi_ec_backlight (when using an AMD APU +
nvidia dgpu).
>>> PMF is meant for AMD APUs(atleast for now) and the _HID will only be
>>> made visible if its AMD laptop. So using amdgpu's native BACKLIGHT_RAW
>>> should be safe, right?
>> Users can pass say acpi_backlight=video and use the acpi_video
>> driver for backlight control instead of the native GPU backlight
>> control.
>>
For now what should be done here is to call
acpi_video_get_backlight_type()
and then translate the return value from this into a backlight-type:
acpi_backlight_video -> BACKLIGHT_FIRMWARE
acpi_backlight_vendor, -> BACKLIGHT_PLATFORM
acpi_backlight_native, -> BACKLIGHT_RAW
acpi_backlight_nvidia_wmi_ec, -> BACKLIGHT_FIRMWARE
acpi_backlight_apple_gmux, -> BACKLIGHT_PLATFORM
>>> I can add this change in the v2, do you insist on this?
>> Insist is a strong word, but I think that it is a good idea to have
>> this. Evenutally it looks like this code will need to either
>> integrate with
>> the drm drivers lot more; or the drm core needs to export some special
>> hooks for this which the PMF code can then call.
>>
>> Actually thinking more about this, I think that the right thing to do
>> here is make some code register brightness control as a cooling device
>> (which I think is already done in some cases) and then have the PMF
>> code use the cooling-device APIs for this.
>>
>> IMHO that would be a much cleaner solution then this hack.
>
> Yeah, fully agree with Hans. This looks like a rather extreme hack to me.
Sure. Let me think in this direction.
>
> Apart from that what exactly is this thing supposed to do? Prevent
> overheating by reducing the brightness?
Yes that can be one of the cases. But the thought process here is to
help OEMs build their own policies.
A policy is combination of inputs and outputs. Inputs are the
conditions that has to be met and outputs are the actions to be set/done.
The output actions come from PMF TA. One example policy apart from the
case you mentioned is:
if ambient light (received from amd_sfh) is low ; reduce the screen
brightness (received from amdgpu) or vice versa.
Thanks,
Shyam
>
> Regards,
> Christian.
>
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>> Thanks,
>>> Shyam
>>>
Also I'm worried about probe order here, this code currently assumes
that the GPU or other backlight driver has loaded before this runs,
which is not necessarily the case.
I think that if the backlight_device_get_by_type() fails this
should be retried say every 10 seconds from some delayed workqueue
for at least a couple of minutes after boot.
Regards,
Hans
> +
> + backlight_device_set_brightness(bd, pmf->brightness);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
> diff --
Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
Am 26.09.23 um 14:56 schrieb Hans de Goede:
Hi,
On 9/26/23 13:24, Shyam Sundar S K wrote:
Hi Hans,
On 9/26/2023 4:05 PM, Hans de Goede wrote:
Hi,
On 9/22/23 19:50, Shyam Sundar S K wrote:
For the Smart PC Solution to fully work, it has to enact to the actions
coming from TA. Add the initial code path for set interface to AMDGPU.
Co-developed-by: Mario Limonciello
Signed-off-by: Mario Limonciello
Signed-off-by: Shyam Sundar S K
---
drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +
drivers/platform/x86/amd/pmf/pmf.h | 2 ++
drivers/platform/x86/amd/pmf/tee-if.c | 19 +--
include/linux/amd-pmf-io.h | 1 +
4 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
index 232d11833ddc..5c567bff0548 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
return 0;
}
EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
+
+int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
+{
+ struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
+ struct amdgpu_device *adev = drm_to_adev(drm_dev);
+ struct backlight_device *bd;
+
+ if (!(adev->flags & AMD_IS_APU)) {
+ DRM_ERROR("PMF-AMDGPU interface not supported\n");
+ return -ENODEV;
+ }
+
+ bd = backlight_device_get_by_type(BACKLIGHT_RAW);
+ if (!bd)
+ return -ENODEV;
This assumes that the backlight is always controller by the amdgpu's
native backlight driver, but it might e.g. also be handled by
eacpi-video or by nvidia_wmi_ec_backlight (when using an AMD APU +
nvidia dgpu).
PMF is meant for AMD APUs(atleast for now) and the _HID will only be
made visible if its AMD laptop. So using amdgpu's native BACKLIGHT_RAW
should be safe, right?
Users can pass say acpi_backlight=video and use the acpi_video
driver for backlight control instead of the native GPU backlight
control.
For now what should be done here is to call acpi_video_get_backlight_type()
and then translate the return value from this into a backlight-type:
acpi_backlight_video -> BACKLIGHT_FIRMWARE
acpi_backlight_vendor, -> BACKLIGHT_PLATFORM
acpi_backlight_native, -> BACKLIGHT_RAW
acpi_backlight_nvidia_wmi_ec, -> BACKLIGHT_FIRMWARE
acpi_backlight_apple_gmux, -> BACKLIGHT_PLATFORM
I can add this change in the v2, do you insist on this?
Insist is a strong word, but I think that it is a good idea to have
this. Evenutally it looks like this code will need to either integrate with
the drm drivers lot more; or the drm core needs to export some special
hooks for this which the PMF code can then call.
Actually thinking more about this, I think that the right thing to do
here is make some code register brightness control as a cooling device
(which I think is already done in some cases) and then have the PMF
code use the cooling-device APIs for this.
IMHO that would be a much cleaner solution then this hack.
Yeah, fully agree with Hans. This looks like a rather extreme hack to me.
Apart from that what exactly is this thing supposed to do? Prevent
overheating by reducing the brightness?
Regards,
Christian.
Regards,
Hans
Thanks,
Shyam
Also I'm worried about probe order here, this code currently assumes
that the GPU or other backlight driver has loaded before this runs,
which is not necessarily the case.
I think that if the backlight_device_get_by_type() fails this
should be retried say every 10 seconds from some delayed workqueue
for at least a couple of minutes after boot.
Regards,
Hans
+
+ backlight_device_set_brightness(bd, pmf->brightness);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
diff --git a/drivers/platform/x86/amd/pmf/pmf.h
b/drivers/platform/x86/amd/pmf/pmf.h
index 9032df4ba48a..ce89cc0daa5a 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -73,6 +73,7 @@
#define PMF_POLICY_STT_SKINTEMP_APU 7
#define PMF_POLICY_STT_SKINTEMP_HS2 8
#define PMF_POLICY_SYSTEM_STATE 9
+#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
#define PMF_POLICY_P3T38
/* TA macros */
@@ -480,6 +481,7 @@ enum ta_pmf_error_type {
};
struct pmf_action_table {
+ unsigned long display_brightness;
enum system_state system_state;
unsigned long spl; /* in mW */
unsigned long sppt; /* in mW */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
b/drivers/platform/x86/amd/pmf/tee-if.c
index 1608996654e8..eef83a4c 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -
Re: [PATCH v3 2/2] backlight: mp3309c: Add support for MPS MP3309C
On Mon, Sep 25, 2023 at 02:26:09PM +0200, Flavio Suligoi wrote:
> diff --git a/drivers/video/backlight/mp3309c.c
> b/drivers/video/backlight/mp3309c.c
> new file mode 100644
> index ..923ac7f7b291
> --- /dev/null
> +++ b/drivers/video/backlight/mp3309c.c
> @@ -0,0 +1,398 @@
> ...
> +static int mp3309c_bl_update_status(struct backlight_device *bl)
> +{
> + struct mp3309c_chip *chip = bl_get_data(bl);
> + int brightness = backlight_get_brightness(bl);
> + struct pwm_state pwmstate;
> + unsigned int analog_val, bits_val;
> + int i, ret;
> +
> + if (chip->pdata->dimming_mode == DIMMING_PWM) {
> + /*
> + * PWM dimming mode
> + */
> + pwm_get_state(chip->pwmd, &pwmstate);
> + pwm_set_relative_duty_cycle(&pwmstate, brightness,
> + chip->pdata->max_brightness);
> + pwmstate.enabled = true;
> + ret = pwm_apply_state(chip->pwmd, &pwmstate);
> + if (ret)
> + return ret;
> +
> + switch (chip->pdata->status) {
> + case FIRST_POWER_ON:
> + case BACKLIGHT_OFF:
> + /*
> + * After 20ms of low pwm signal level, the chip turns
> +off automatically. In this case, before enabling the
> +chip again, we must wait about 10ms for pwm signal to
> +stabilize.
> + */
> + if (brightness > 0) {
> + msleep(10);
> + mp3309c_enable_device(chip);
> + chip->pdata->status = BACKLIGHT_ON;
> + } else {
> + chip->pdata->status = BACKLIGHT_OFF;
> + }
> + break;
> + case BACKLIGHT_ON:
> + if (brightness == 0)
> + chip->pdata->status = BACKLIGHT_OFF;
> + break;
> + }
> + } else {
> + /*
> + * Analog dimming (by I2C command) dimming mode
> + *
> + * The first time, before setting brightness, we must enable the
> + * device
> + */
> + if (chip->pdata->status == FIRST_POWER_ON)
> + mp3309c_enable_device(chip);
> +
> + /*
> + * Dimming mode I2C command
> + *
> + * The 5 bits of the dimming analog value D4..D0 is allocated
> + * in the I2C register #0, in the following way:
> + *
> + * +--+--+--+--+--+--+--+--+
> + * |EN|D0|D1|D2|D3|D4|XX|XX|
> + * +--+--+--+--+--+--+--+--+
> + */
> + analog_val = DIV_ROUND_UP(ANALOG_MAX_VAL * brightness,
> + chip->pdata->max_brightness);
Sorry to only notice after sharing a Reviewed-by:[1] but...
Scaling brightness here isn't right. When running in I2C dimming mode then
max_brightness *must* be 31 or lower, meaning the value in brightness can
be applied directly to the hardware without scaling.
Quoting the DT binding docs about how max-brightness should be
interpretted:
Normally the maximum brightness is determined by the hardware and this
property is not required. This property is used to put a software
limit on the brightness apart from what the driver says, as it could
happen that a LED can be made so bright that it gets damaged or causes
damage due to restrictions in a specific system, such as mounting
conditions.
Daniel.
[1] I remember checking if this code could overflow the field but I was
so distracted by that I ended up missing the obvious!
Re: [PATCH 2/2] drm/amd/display: Fix null pointer dereference in error message
On 2023-09-26 01:56, Cong Liu wrote:
> This patch fixes a null pointer dereference in the error message that is
> printed when the Display Core (DC) fails to initialize. The original
> message includes the DC version number, which is undefined if the DC is
> not initialized.
>
> Fixes: 9788d087caff ("drm/amd/display: improve the message printed when
> loading DC")
> Signed-off-by: Cong Liu
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 8e98dda1e084..bf52a909f558 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1703,8 +1703,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
> DRM_INFO("Display Core v%s initialized on %s\n", DC_VER,
>dce_version_to_string(adev->dm.dc->ctx->dce_version));
> } else {
> - DRM_INFO("Display Core v%s failed to initialize on %s\n",
> DC_VER,
> - dce_version_to_string(adev->dm.dc->ctx->dce_version));
> + DRM_INFO("Display Core failed to initialize with v%s!\n",
> DC_VER);
There is value in printing the version number. Let's not remove it.
Instead you can probably fix it by doing a NULL check on adev->dm.dc->ctx.
Harry
> goto error;
> }
>
Re: [PATCH] drm/amd/display: set stream gamut remap matrix to MPC for DCN3+
On 09/25, Harry Wentland wrote:
>
>
> On 2023-07-21 09:24, Melissa Wen wrote:
> > dc->caps.color.mpc.gamut_remap says there is a post-blending color block
> > for gamut remap matrix for DCN3 HW family and newer versions. However,
> > those drivers still follow DCN10 programming that remap stream
> > gamut_remap_matrix to DPP (pre-blending).
> >
> > To enable pre-blending and post-blending gamut_remap matrix supports at
> > the same time, set stream gamut_remap to MPC and plane gamut_remap to
> > DPP for DCN families that support both.
> >
> > It was tested using IGT KMS color tests for DRM CRTC CTM property and it
> > preserves test results.
> >
> > Signed-off-by: Melissa Wen
> >
>
> Reviewed-by: Harry Wentland
>
> and merged.
>
> I also took the liberty to expand this to the recently merged dcn35
> code.
No problem. Thank you!
With this change merged, I also removed the related patch from the
series of AMD driver-specific color props.
Melissa
>
> Harry
>
> > ---
> >
> > Hi,
> >
> > Two relevant things to consider for this change. One is that mapping DRM
> > CRTC CTM to MPC is a more consistent way since CRTC CTM is a
> > post-blending transformation. Second, programming stream
> > gamut_remap_matrix on MPC enables us to provide support for both plane
> > CTM and CRTC CTM color properties. If we don't make this separation, we
> > would need to reject an atomic commit that tries to set both properties
> > at the same time and userspace may also get unexpected results.
> >
> > Thanks in advance for any feeback,
> >
> > Melissa
> >
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.c| 37 +++
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.h| 3 ++
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 2 +-
> > .../drm/amd/display/dc/dcn301/dcn301_init.c | 2 +-
> > .../gpu/drm/amd/display/dc/dcn31/dcn31_init.c | 2 +-
> > .../drm/amd/display/dc/dcn314/dcn314_init.c | 2 +-
> > .../gpu/drm/amd/display/dc/dcn32/dcn32_init.c | 2 +-
> > 7 files changed, 45 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > index 4cd4ae07d73d..4fb4e9ec03f1 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > @@ -186,6 +186,43 @@ bool dcn30_set_input_transfer_func(struct dc *dc,
> > return result;
> > }
> >
> > +void dcn30_program_gamut_remap(struct pipe_ctx *pipe_ctx)
> > +{
> > + int i = 0;
> > + struct dpp_grph_csc_adjustment dpp_adjust;
> > + struct mpc_grph_gamut_adjustment mpc_adjust;
> > + int mpcc_id = pipe_ctx->plane_res.hubp->inst;
> > + struct mpc *mpc = pipe_ctx->stream_res.opp->ctx->dc->res_pool->mpc;
> > +
> > + memset(&dpp_adjust, 0, sizeof(dpp_adjust));
> > + dpp_adjust.gamut_adjust_type = GRAPHICS_GAMUT_ADJUST_TYPE_BYPASS;
> > +
> > + if (pipe_ctx->plane_state &&
> > + pipe_ctx->plane_state->gamut_remap_matrix.enable_remap == true) {
> > + dpp_adjust.gamut_adjust_type = GRAPHICS_GAMUT_ADJUST_TYPE_SW;
> > + for (i = 0; i < CSC_TEMPERATURE_MATRIX_SIZE; i++)
> > + dpp_adjust.temperature_matrix[i] =
> > +
> > pipe_ctx->plane_state->gamut_remap_matrix.matrix[i];
> > + }
> > +
> > +
> > pipe_ctx->plane_res.dpp->funcs->dpp_set_gamut_remap(pipe_ctx->plane_res.dpp,
> > + &dpp_adjust);
> > +
> > + memset(&mpc_adjust, 0, sizeof(mpc_adjust));
> > + mpc_adjust.gamut_adjust_type = GRAPHICS_GAMUT_ADJUST_TYPE_BYPASS;
> > +
> > + if (pipe_ctx->top_pipe == NULL) {
> > + if (pipe_ctx->stream->gamut_remap_matrix.enable_remap == true) {
> > + mpc_adjust.gamut_adjust_type =
> > GRAPHICS_GAMUT_ADJUST_TYPE_SW;
> > + for (i = 0; i < CSC_TEMPERATURE_MATRIX_SIZE; i++)
> > + mpc_adjust.temperature_matrix[i] =
> > +
> > pipe_ctx->stream->gamut_remap_matrix.matrix[i];
> > + }
> > + }
> > +
> > + mpc->funcs->set_gamut_remap(mpc, mpcc_id, &mpc_adjust);
> > +}
> > +
> > bool dcn30_set_output_transfer_func(struct dc *dc,
> > struct pipe_ctx *pipe_ctx,
> > const struct dc_stream_state *stream)
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > index a24a8e33a3d2..cb34ca932a5f 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> > @@ -58,6 +58,9 @@ bool dcn30_set_blend_lut(struct pipe_ctx *pipe_ctx,
> > bool dcn30_set_input_transfer_func(struct dc *dc,
> > struct pipe_ctx *pipe_ctx,
> > const struct dc_plane_state *plane_state);
> > +
> > +void dcn30_program_gamut_remap(s
Re: [PATCH v3 1/2] dt-bindings: backlight: Add MPS MP3309C
On Mon, Sep 25, 2023 at 02:26:08PM +0200, Flavio Suligoi wrote: > The Monolithic Power (MPS) MP3309C is a WLED step-up converter, featuring a > programmable switching frequency to optimize efficiency. > The brightness can be controlled either by I2C commands (called "analog" > mode) or by a PWM input signal (PWM mode). > This driver supports both modes. > > For device driver details, please refer to: > - drivers/video/backlight/mp3309c_bl.c > > The datasheet is available at: > - https://www.monolithicpower.com/en/mp3309c.html > > Signed-off-by: Flavio Suligoi > Reviewed-by: Rob Herring > --- > > v3: > - add default value for mps,overvoltage-protection-microvolt property > - fix the example, changing from "mps,mp3309c-backlight" to "mps,mp3309c" in >compatible property > v2: > - remove useless properties (dimming-mode, pinctrl-names, pinctrl-0, >switch-on-delay-ms, switch-off-delay-ms, reset-gpios, reset-on-delay-ms, >reset-on-length-ms) > - add common.yaml# > - remove already included properties (default-brightness, max-brightness) > - substitute three boolean properties, used for the overvoltage-protection >values, with a single enum property > - remove some conditional definitions > - remove the 2nd example > v1: > - first version > > .../bindings/leds/backlight/mps,mp3309c.yaml | 73 +++ > 1 file changed, 73 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml > > diff --git > a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml > b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml > new file mode 100644 > index ..4191e33626f5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml > @@ -0,0 +1,73 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/backlight/mps,mp3309c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MPS MP3309C backlight > + > +maintainers: > + - Flavio Suligoi > + > +description: | > + The Monolithic Power (MPS) MP3309C is a WLED step-up converter, featuring a > + programmable switching frequency to optimize efficiency. > + It supports two different dimming modes: > + > + - analog mode, via I2C commands (default) > + - PWM controlled mode. > + > + The datasheet is available at: > + https://www.monolithicpower.com/en/mp3309c.html > + > +allOf: > + - $ref: common.yaml# > + > +properties: > + compatible: > +const: mps,mp3309c > + > + reg: > +maxItems: 1 > + > + pwms: > +description: if present, the backlight is controlled in PWM mode. > +maxItems: 1 > + > + enable-gpios: > +description: GPIO used to enable the backlight in "analog-i2c" dimming > mode. > +maxItems: 1 > + > + mps,overvoltage-protection-microvolt: > +description: Overvoltage protection (13.5V, 24V or 35.5V). > +enum: [ 1350, 2400, 3550 ] > +default: 3550 > + > + mps,no-sync-mode: > +description: disable synchronous rectification mode > +type: boolean > + > +required: > + - compatible > + - reg > + - max-brightness Why is this mandatory? There's no point in setting max-brightness when running in I2C mode (max-brightness should default to 31 in that case). > + - default-brightness Again. I'm not clear why this needs to be mandatory. Daniel.
Re: [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state
On 2023-09-26 08:38, Melissa Wen wrote:
> On 09/25, Harry Wentland wrote:
>>
>>
>> On 2023-09-13 12:43, Melissa Wen wrote:
>>> Logging DCN3 MPC state was following DCN1 implementation that doesn't
>>> consider new DCN3 MPC color blocks. Create new elements according to
>>> DCN3 MPC color caps and a new DCN3-specific function for reading MPC
>>> data.
>>>
>>> Signed-off-by: Melissa Wen
>>> ---
>>> .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c | 55 ++-
>>> drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 13 +
>>> 2 files changed, 67 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> index d1500b223858..d164fbf89212 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> @@ -1382,8 +1382,61 @@ static void mpc3_set_mpc_mem_lp_mode(struct mpc *mpc)
>>> }
>>> }
>>>
>>> +static void mpc3_read_mpcc_state(
>>> + struct mpc *mpc,
>>> + int mpcc_inst,
>>> + struct mpcc_state *s)
>>> +{
>>> + struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
>>> + uint32_t rmu_status = 0xf;
>>> +
>>> + REG_GET(MPCC_OPP_ID[mpcc_inst], MPCC_OPP_ID, &s->opp_id);
>>> + REG_GET(MPCC_TOP_SEL[mpcc_inst], MPCC_TOP_SEL, &s->dpp_id);
>>> + REG_GET(MPCC_BOT_SEL[mpcc_inst], MPCC_BOT_SEL, &s->bot_mpcc_id);
>>> + REG_GET_4(MPCC_CONTROL[mpcc_inst], MPCC_MODE, &s->mode,
>>> + MPCC_ALPHA_BLND_MODE, &s->alpha_mode,
>>> + MPCC_ALPHA_MULTIPLIED_MODE, &s->pre_multiplied_alpha,
>>> + MPCC_BLND_ACTIVE_OVERLAP_ONLY, &s->overlap_only);
>>> + REG_GET_2(MPCC_STATUS[mpcc_inst], MPCC_IDLE, &s->idle,
>>> + MPCC_BUSY, &s->busy);
>>> +
>>> + /* Color blocks state */
>>> + REG_GET(MPC_RMU_CONTROL, MPC_RMU0_MUX_STATUS, &rmu_status);
>>> + if (rmu_status == mpcc_inst) {
>>> + REG_GET(SHAPER_CONTROL[0],
>>> + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
>>> + REG_GET(RMU_3DLUT_MODE[0],
>>> + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
>>> + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[0],
>>> + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
>>> + REG_GET(RMU_3DLUT_MODE[0],
>>> + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
>>> + } else {
>>> + REG_GET(SHAPER_CONTROL[1],
>>> + MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
>>> + REG_GET(RMU_3DLUT_MODE[1],
>>> + MPC_RMU_3DLUT_MODE_CURRENT, &s->lut3d_mode);
>>> + REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[1],
>>> + MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
>>> + REG_GET(RMU_3DLUT_MODE[1],
>>> + MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
>>> + }
>>> + REG_GET_2(MPCC_OGAM_CONTROL[mpcc_inst],
>>> + MPCC_OGAM_MODE_CURRENT, &s->rgam_mode,
>>> + MPCC_OGAM_SELECT_CURRENT, &s->rgam_lut);
>>> + REG_GET(MPCC_GAMUT_REMAP_MODE[mpcc_inst],
>>> + MPCC_GAMUT_REMAP_MODE_CURRENT, &s->gamut_remap_mode);
>>> + if (s->gamut_remap_mode == 1) {
>>> + s->gamut_remap_c11_c12 =
>>> REG_READ(MPC_GAMUT_REMAP_C11_C12_A[mpcc_inst]);
>>> + s->gamut_remap_c33_c34 =
>>> REG_READ(MPC_GAMUT_REMAP_C33_C34_A[mpcc_inst]);
>>> + } else if (s->gamut_remap_mode == 2) {
>>> + s->gamut_remap_c11_c12 =
>>> REG_READ(MPC_GAMUT_REMAP_C11_C12_B[mpcc_inst]);
>>> + s->gamut_remap_c33_c34 =
>>> REG_READ(MPC_GAMUT_REMAP_C33_C34_B[mpcc_inst]);
>>
>> Any reason we're getting (and printing) only the first and last
>> coefficients? Is it to avoid printing lines that are too wide?
>
> I'm unable to reach the other coefficients through this
> `MPC_GAMUT_REMAP` register prefix. But I'm probably missing something
> since I can see the registers using UMR. I'll try to find the right path
> and update it.
>
>From dcn3_0_1_offset.h the registers are there:
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_A
> 0x014c
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_A_BASE_IDX
> 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_A
> 0x014d
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_A_BASE_IDX
> 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_A
> 0x014e
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_A_BASE_IDX
> 3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_A
> 0x014f
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_A_BASE_IDX
> 3
> #d
Re: [PATCH 2/2] dt-bindings: display: msm: Make "additionalProperties: true" explicit
On Mon, Sep 25, 2023 at 04:24:25PM -0500, Rob Herring wrote: > Make it explicit that child nodes have additional properties and the > child node schema is not complete. The complete schemas are applied > separately based the compatible strings. > > Signed-off-by: Rob Herring I cross-checked only a handful of these... Acked-by: Conor Dooley Thanks, Conor. > --- > .../bindings/display/msm/qcom,msm8998-mdss.yaml| 6 ++ > .../bindings/display/msm/qcom,qcm2290-mdss.yaml| 6 ++ > .../bindings/display/msm/qcom,sc7180-mdss.yaml | 8 > .../bindings/display/msm/qcom,sc7280-mdss.yaml | 10 ++ > .../bindings/display/msm/qcom,sc8280xp-mdss.yaml | 4 > .../bindings/display/msm/qcom,sdm845-mdss.yaml | 8 > .../bindings/display/msm/qcom,sm6115-mdss.yaml | 6 ++ > .../bindings/display/msm/qcom,sm6125-mdss.yaml | 6 ++ > .../bindings/display/msm/qcom,sm6350-mdss.yaml | 6 ++ > .../bindings/display/msm/qcom,sm6375-mdss.yaml | 6 ++ > .../bindings/display/msm/qcom,sm8150-mdss.yaml | 6 ++ > .../bindings/display/msm/qcom,sm8250-mdss.yaml | 6 ++ > .../bindings/display/msm/qcom,sm8350-mdss.yaml | 8 > .../bindings/display/msm/qcom,sm8450-mdss.yaml | 8 > .../bindings/display/msm/qcom,sm8550-mdss.yaml | 8 > 15 files changed, 102 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/msm/qcom,msm8998-mdss.yaml > b/Documentation/devicetree/bindings/display/msm/qcom,msm8998-mdss.yaml > index e320ab1de6de..2d9edab5a30d 100644 > --- a/Documentation/devicetree/bindings/display/msm/qcom,msm8998-mdss.yaml > +++ b/Documentation/devicetree/bindings/display/msm/qcom,msm8998-mdss.yaml > @@ -38,12 +38,16 @@ properties: > patternProperties: >"^display-controller@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > const: qcom,msm8998-dpu > >"^dsi@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > items: > @@ -52,6 +56,8 @@ patternProperties: > >"^phy@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > const: qcom,dsi-phy-10nm-8998 > diff --git > a/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml > b/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml > index 4184b84d4c21..5ad155612b6c 100644 > --- a/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml > +++ b/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml > @@ -44,18 +44,24 @@ properties: > patternProperties: >"^display-controller@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > const: qcom,qcm2290-dpu > >"^dsi@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > const: qcom,dsi-ctrl-6g-qcm2290 > >"^phy@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > const: qcom,dsi-phy-14nm-2290 > diff --git > a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml > b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml > index 3b9c103e504a..3432a2407caa 100644 > --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml > @@ -44,18 +44,24 @@ properties: > patternProperties: >"^display-controller@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > const: qcom,sc7180-dpu > >"^displayport-controller@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > const: qcom,sc7180-dp > >"^dsi@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > items: > @@ -64,6 +70,8 @@ patternProperties: > >"^phy@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > const: qcom,dsi-phy-10nm > diff --git > a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml > b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml > index 43500dad66e7..bbb727831fca 100644 > --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml > @@ -44,18 +44,24 @@ properties: > patternProperties: >"^display-controller@[0-9a-f]+$": > type: object > +additionalProperties: true > + > properties: >compatible: > const: qco
Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
Hi,
On 9/26/23 13:24, Shyam Sundar S K wrote:
> Hi Hans,
>
> On 9/26/2023 4:05 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 9/22/23 19:50, Shyam Sundar S K wrote:
>>> For the Smart PC Solution to fully work, it has to enact to the actions
>>> coming from TA. Add the initial code path for set interface to AMDGPU.
>>>
>>> Co-developed-by: Mario Limonciello
>>> Signed-off-by: Mario Limonciello
>>> Signed-off-by: Shyam Sundar S K
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +
>>> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
>>> drivers/platform/x86/amd/pmf/tee-if.c | 19 +--
>>> include/linux/amd-pmf-io.h | 1 +
>>> 4 files changed, 41 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> index 232d11833ddc..5c567bff0548 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
>>> return 0;
>>> }
>>> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>> +
>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
>>> +{
>>> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>> + struct backlight_device *bd;
>>> +
>>> + if (!(adev->flags & AMD_IS_APU)) {
>>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>> + if (!bd)
>>> + return -ENODEV;
>>
>> This assumes that the backlight is always controller by the amdgpu's
>> native backlight driver, but it might e.g. also be handled by
>> eacpi-video or by nvidia_wmi_ec_backlight (when using an AMD APU +
>> nvidia dgpu).
>
> PMF is meant for AMD APUs(atleast for now) and the _HID will only be
> made visible if its AMD laptop. So using amdgpu's native BACKLIGHT_RAW
> should be safe, right?
Users can pass say acpi_backlight=video and use the acpi_video
driver for backlight control instead of the native GPU backlight
control.
>
>>
>> For now what should be done here is to call acpi_video_get_backlight_type()
>> and then translate the return value from this into a backlight-type:
>>
>> acpi_backlight_video -> BACKLIGHT_FIRMWARE
>> acpi_backlight_vendor, -> BACKLIGHT_PLATFORM
>> acpi_backlight_native, -> BACKLIGHT_RAW
>> acpi_backlight_nvidia_wmi_ec,-> BACKLIGHT_FIRMWARE
>> acpi_backlight_apple_gmux, -> BACKLIGHT_PLATFORM
>>
>
> I can add this change in the v2, do you insist on this?
Insist is a strong word, but I think that it is a good idea to have
this. Evenutally it looks like this code will need to either integrate with
the drm drivers lot more; or the drm core needs to export some special
hooks for this which the PMF code can then call.
Actually thinking more about this, I think that the right thing to do
here is make some code register brightness control as a cooling device
(which I think is already done in some cases) and then have the PMF
code use the cooling-device APIs for this.
IMHO that would be a much cleaner solution then this hack.
Regards,
Hans
>
> Thanks,
> Shyam
>
>> Also I'm worried about probe order here, this code currently assumes
>> that the GPU or other backlight driver has loaded before this runs,
>> which is not necessarily the case.
>>
>> I think that if the backlight_device_get_by_type() fails this
>> should be retried say every 10 seconds from some delayed workqueue
>> for at least a couple of minutes after boot.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>>> +
>>> + backlight_device_set_brightness(bd, pmf->brightness);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>>> b/drivers/platform/x86/amd/pmf/pmf.h
>>> index 9032df4ba48a..ce89cc0daa5a 100644
>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>> @@ -73,6 +73,7 @@
>>> #define PMF_POLICY_STT_SKINTEMP_APU7
>>> #define PMF_POLICY_STT_SKINTEMP_HS28
>>> #define PMF_POLICY_SYSTEM_STATE9
>>> +#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
>>> #define PMF_POLICY_P3T 38
>>>
>>> /* TA macros */
>>> @@ -480,6 +481,7 @@ enum ta_pmf_error_type {
>>> };
>>>
>>> struct pmf_action_table {
>>> + unsigned long display_brightness;
>>> enum system_state system_state;
>>> unsigned long spl; /* in mW */
>>> unsigned long sppt; /* in mW */
>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>>> b/drivers/platform/x86/amd/pmf/tee-if.c
>>> index 1608996654e8..eef83a4c 100644
>>> --- a/drivers/platform/x86/amd/pmf/
Re: [PATCH 1/2] dt-bindings: display: msm: Add missing unevaluatedProperties on child node schemas
On Mon, Sep 25, 2023 at 04:24:24PM -0500, Rob Herring wrote: > Just as unevaluatedProperties or additionalProperties are required at > the top level of schemas, they should (and will) also be required for > child node schemas. That ensures only documented properties are > present for any node. > > Signed-off-by: Rob Herring Acked-by: Conor Dooley Thanks, Conor. > --- > Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > index f12558960cd8..dbe398f84ffb 100644 > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > @@ -114,6 +114,7 @@ properties: > >port@1: > $ref: /schemas/graph.yaml#/$defs/port-base > +unevaluatedProperties: false > description: Output endpoint of the controller > properties: >endpoint: > -- > 2.40.1 > signature.asc Description: PGP signature
Re: [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log
On 09/25, Harry Wentland wrote: > > > On 2023-09-13 12:43, Melissa Wen wrote: > > Hi, > > > > This is an update of previous RFC [0] improving the data collection of > > Gamma Correction and Blend Gamma color blocks. > > > > As I mentioned in the last version, I'm updating the color state part of > > DTN log to match DCN3.0 HW better. Currently, the DTN log considers the > > DCN10 color pipeline, which is useless for DCN3.0 because of all the > > differences in color caps between DCN versions. In addition to new color > > blocks and caps, some semantic differences made the DCN10 output not fit > > DCN30. > > > > In this RFC, the first patch adds new color state elements to DPP and > > implements the reading of registers according to HW blocks. Similarly to > > MPC, the second patch also creates a DCN3-specific function to read the > > MPC state and add the MPC color state logging to it. With DPP and MPC > > color-register reading, I detach DCN10 color state logging from the HW > > log and create a `.log_color_state` hook for logging color state > > according to HW color blocks with DCN30 as the first use case. Finally, > > the last patch adds DPP and MPC color caps output to facilitate > > understanding of the color state log. > > > > This version works well with the driver-specific color properties[1] and > > steamdeck/gamescope[2] together, where we can see color state changing > > from default values. > > > > Here is a before vs. after example: > > > > Without this series: > > === > > DPP:IGAM format IGAM modeDGAM modeRGAM mode GAMUT mode C11 > > C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34 > > [ 0]:0h BypassFixed Bypass Bypass0 > > h h h h h h > > [ 3]:0h BypassFixed Bypass Bypass0 > > h h h h h h > > > > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE > > [ 0]: 0h 0h 3h 3 20 0 0 > > [ 3]: 0h 3h fh 2 20 0 0 > > > > With this series (Steamdeck/Gamescope): > > == > > > > DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit > > depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 > > C24 C31 C32 C33 C34 > > [ 0]:1 sRGBBypassRAM A RAM B > > 12-bit17x17x17 RAM A 0 h h h > > h h h > > [ 1]:1 sRGBBypassRAM B RAM A > > 12-bit17x17x17 RAM A 0 h h h > > h h h > > [ 2]:1 sRGBBypassRAM B RAM A > > 12-bit17x17x17 RAM A 0 h h h > > h h h > > [ 3]:1 sRGBBypassRAM A RAM B > > 12-bit17x17x17 RAM A 0 h h h > > h h h > > > > DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: > > srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 > > dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0 > > > > MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE > > SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT > > GAMUT mode C11 C12 C33 C34 > > [ 0]: 0h 0h 2h 3 01 0 0 > > Bypass Bypass 12-bit17x17x17RAM A > > 0 h h > > [ 1]: 0h 1h fh 2 20 0 0 > > Bypass Bypass 12-bit17x17x17 Bypass A > > 0 h h > > [ 2]: 0h 2h 3h 3 01 0 0 > > Bypass Bypass 12-bit17x17x17 Bypass A > > 0 h h > > [ 3]: 0h 3h 1h 3 20 0 0 > > Bypass Bypass 12-bit17x17x17 Bypass A > > 0 h h > > > > MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1 > > > > With this series (Steamdeck/KDE): > > > > > > DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit > > depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 > > C24 C31 C32 C33 C34 > > [ 0]:0 sRGBBypass Bypass Bypass > > 12-bit 9x9x9 Bypass 0 h h h > > h h h > > [ 3]:0 sRGBBypass Bypass Byp
Re: [RFC PATCH v2 4/5] drm/amd/display: hook DCN30 color state logging to DTN log
On 09/25, Harry Wentland wrote:
>
>
> On 2023-09-13 12:43, Melissa Wen wrote:
> > Color caps changed between HW versions which caused DCN10 color state
> > sections on DTN log no longer fit DCN3.0 versions. Create a
> > DCN3.0-specific color state logging and hook it to drivers of DCN3.0
> > family.
> >
> > rfc-v2:
> > - detail RAM mode for gamcor and blnd gamma blocks
> >
> > Signed-off-by: Melissa Wen
> > ---
> > .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 5 +-
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.c| 88 +++
> > .../drm/amd/display/dc/dcn30/dcn30_hwseq.h| 3 +
> > .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 1 +
> > .../drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
> > .../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +
> > 6 files changed, 99 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > index a0c489ed218c..9255425ef794 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> > @@ -358,7 +358,10 @@ void dcn10_log_hw_state(struct dc *dc,
> >
> > dcn10_log_hubp_states(dc, log_ctx);
> >
> > - dcn10_log_color_state(dc, log_ctx);
> > + if (dc->hwss.log_color_state)
> > + dc->hwss.log_color_state(dc, log_ctx);
> > + else
> > + dcn10_log_color_state(dc, log_ctx);
> >
> > DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel
> > vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n");
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > index 255713ec29bb..47119ae80e98 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > @@ -69,6 +69,94 @@
> > #define FN(reg_name, field_name) \
> > hws->shifts->field_name, hws->masks->field_name
> >
> > +void
> > +dcn30_log_color_state(struct dc *dc,
> > + struct dc_log_buffer_ctx *log_ctx)
> > +{
> > + struct dc_context *dc_ctx = dc->ctx;
> > + struct resource_pool *pool = dc->res_pool;
> > + int i;
> > +
> > + DTN_INFO("DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode"
> > +" 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode"
> > +" GAMUT mode "
> > +"C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34\n");
> > +
> > + for (i = 0; i < pool->pipe_count; i++) {
> > + struct dpp *dpp = pool->dpps[i];
> > + struct dcn_dpp_state s = {0};
> > +
> > + dpp->funcs->dpp_read_state(dpp, &s);
> > +
> > + if (!s.is_enabled)
> > + continue;
> > +
> > + DTN_INFO("[%2d]: %7x %13s %8s %11s %10s %15s %10s %9s"
> > +" %10x %08xh %08xh %08xh %08xh %08xh %08xh",
> > + dpp->inst,
> > + s.pre_dgam_mode,
> > + (s.pre_dgam_select == 0) ? "sRGB" :
> > +((s.pre_dgam_select == 1) ? "Gamma 2.2" :
> > +((s.pre_dgam_select == 2) ? "Gamma 2.4" :
> > +((s.pre_dgam_select == 3) ? "Gamma 2.6" :
> > +((s.pre_dgam_select == 4) ? "BT.709" :
> > +((s.pre_dgam_select == 5) ? "PQ" :
> > +((s.pre_dgam_select == 6) ? "HLG" :
> > +"Unknown")),
> > + (s.gamcor_mode == 0) ? "Bypass" :
> > +((s.gamcor_mode == 1) ? "RAM A" :
> > +"RAM B"),
> > + (s.shaper_lut_mode == 1) ? "RAM A" :
> > +((s.shaper_lut_mode == 2) ? "RAM B" :
> > +"Bypass"),
> > + (s.lut3d_mode == 1) ? "RAM A" :
> > +((s.lut3d_mode == 2) ? "RAM B" :
> > + "Bypass"),
> > + (s.lut3d_bit_depth <= 0) ? "12-bit" : "10-bit",
> > + (s.lut3d_size == 0) ? "17x17x17" : "9x9x9",
> > + (s.rgam_lut_mode == 0) ? "Bypass" :
> > +((s.rgam_lut_mode == 1) ? "RAM A" :
> > + "RAM B"),
> > + s.gamut_remap_mode,
> > + s.gamut_remap_c11_c12, s.gamut_remap_c13_c14,
> > + s.gamut_remap_c21_c22, s.gamut_remap_c23_c24,
> > + s.gamut_remap_c31_c32, s.gamut_remap_c33_c34);
> > + DTN_INFO("\n");
> > + }
> > + DTN_INFO("\n");
> > +
> > + DTN_INFO("MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT
> > OVERLAP_ONLY IDLE"
> > +" SHAPER mode 3DLUT mode 3DLUT bit-depth 3DLUT size OGAM
> > mode OGAM LUT"
> > +"
Re: [PATCH 0/8] Add DSC fractional bpp support
Hi, For coding style and wording part, this version looks fine for me after a brief skim. Thanks for the patch. :-) On 2023/9/26 16:23, Mitul Golani wrote: This patch series adds support for DSC fractional compressed bpp for MTL+. The series starts with some fixes, followed by patches that lay groundwork to iterate over valid compressed bpps to select the 'best' compressed bpp with optimal link configuration (taken from upstream series: https://patchwork.freedesktop.org/series/105200/). The later patches, add changes to accommodate compressed bpp with fractional part, including changes to QP calculations. To get the 'best' compressed bpp, we iterate over the valid compressed bpp values, but with fractional step size 1/16, 1/8, 1/4 or 1/2 as per sink support. The last 2 patches add support to depict DSC sink's fractional support, and debugfs to enforce use of fractional bpp, while choosing an appropriate compressed bpp. Ankit Nautiyal (5): drm/display/dp: Add helper function to get DSC bpp precision drm/i915/display: Store compressed bpp in U6.4 format drm/i915/display: Consider fractional vdsc bpp while computing m_n values drm/i915/audio : Consider fractional vdsc bpp while computing tu_data drm/i915/dp: Iterate over output bpp with fractional step size Swati Sharma (2): drm/i915/dsc: Add debugfs entry to validate DSC fractional bpp drm/i915/dsc: Allow DSC only with fractional bpp when forced from debugfs Vandita Kulkarni (1): drm/i915/dsc/mtl: Add support for fractional bpp drivers/gpu/drm/display/drm_dp_helper.c | 27 ++ drivers/gpu/drm/i915/display/icl_dsi.c| 11 +-- drivers/gpu/drm/i915/display/intel_audio.c| 17 ++-- drivers/gpu/drm/i915/display/intel_bios.c | 6 +- drivers/gpu/drm/i915/display/intel_cdclk.c| 6 +- drivers/gpu/drm/i915/display/intel_display.c | 8 +- drivers/gpu/drm/i915/display/intel_display.h | 2 +- .../drm/i915/display/intel_display_debugfs.c | 84 +++ .../drm/i915/display/intel_display_types.h| 4 +- drivers/gpu/drm/i915/display/intel_dp.c | 81 +++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 32 --- drivers/gpu/drm/i915/display/intel_fdi.c | 2 +- .../i915/display/intel_fractional_helper.h| 36 .../gpu/drm/i915/display/intel_qp_tables.c| 3 - drivers/gpu/drm/i915/display/intel_vdsc.c | 30 +-- include/drm/display/drm_dp_helper.h | 1 + 16 files changed, 276 insertions(+), 74 deletions(-) create mode 100644 drivers/gpu/drm/i915/display/intel_fractional_helper.h
