Re: [PATCH v3 1/4] drm/amdgpu: add or move defines for DCE6 in sid.h
On Thu, Mar 6, 2025 at 10:19 AM Alex Deucher wrote:
>
> On Wed, Mar 5, 2025 at 9:08 PM Alexandre Demers
> wrote:
> >
> > For coherence with DCE8 et DCE10, add or move some values under sid.h.
> >
> > Signed-off-by: Alexandre Demers
>
> This change doesn't build. Please adjust the order of the patches as
> needed to make sure they all build.
>
> Alex
Yeah, adding sid.h should probably be at the end, once all changes are
made. I'll look at it.
Thanks for noticing.
Alexandre
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 63 ++-
> > drivers/gpu/drm/amd/amdgpu/si_enums.h | 7 ---
> > drivers/gpu/drm/amd/amdgpu/sid.h | 29 +---
> > 3 files changed, 55 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > index a72fd7220081..185401d66961 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > @@ -32,6 +32,7 @@
> > #include "amdgpu.h"
> > #include "amdgpu_pm.h"
> > #include "amdgpu_i2c.h"
> > +#include "sid.h"
> > #include "atom.h"
> > #include "amdgpu_atombios.h"
> > #include "atombios_crtc.h"
> > @@ -59,31 +60,31 @@ static void dce_v6_0_set_irq_funcs(struct amdgpu_device
> > *adev);
> >
> > static const u32 crtc_offsets[6] =
> > {
> > - SI_CRTC0_REGISTER_OFFSET,
> > - SI_CRTC1_REGISTER_OFFSET,
> > - SI_CRTC2_REGISTER_OFFSET,
> > - SI_CRTC3_REGISTER_OFFSET,
> > - SI_CRTC4_REGISTER_OFFSET,
> > - SI_CRTC5_REGISTER_OFFSET
> > + CRTC0_REGISTER_OFFSET,
> > + CRTC1_REGISTER_OFFSET,
> > + CRTC2_REGISTER_OFFSET,
> > + CRTC3_REGISTER_OFFSET,
> > + CRTC4_REGISTER_OFFSET,
> > + CRTC5_REGISTER_OFFSET
> > };
> >
> > static const u32 hpd_offsets[] =
> > {
> > - mmDC_HPD1_INT_STATUS - mmDC_HPD1_INT_STATUS,
> > - mmDC_HPD2_INT_STATUS - mmDC_HPD1_INT_STATUS,
> > - mmDC_HPD3_INT_STATUS - mmDC_HPD1_INT_STATUS,
> > - mmDC_HPD4_INT_STATUS - mmDC_HPD1_INT_STATUS,
> > - mmDC_HPD5_INT_STATUS - mmDC_HPD1_INT_STATUS,
> > - mmDC_HPD6_INT_STATUS - mmDC_HPD1_INT_STATUS,
> > + HPD0_REGISTER_OFFSET,
> > + HPD1_REGISTER_OFFSET,
> > + HPD2_REGISTER_OFFSET,
> > + HPD3_REGISTER_OFFSET,
> > + HPD4_REGISTER_OFFSET,
> > + HPD5_REGISTER_OFFSET
> > };
> >
> > static const uint32_t dig_offsets[] = {
> > - SI_CRTC0_REGISTER_OFFSET,
> > - SI_CRTC1_REGISTER_OFFSET,
> > - SI_CRTC2_REGISTER_OFFSET,
> > - SI_CRTC3_REGISTER_OFFSET,
> > - SI_CRTC4_REGISTER_OFFSET,
> > - SI_CRTC5_REGISTER_OFFSET,
> > + CRTC0_REGISTER_OFFSET,
> > + CRTC1_REGISTER_OFFSET,
> > + CRTC2_REGISTER_OFFSET,
> > + CRTC3_REGISTER_OFFSET,
> > + CRTC4_REGISTER_OFFSET,
> > + CRTC5_REGISTER_OFFSET,
> > (0x13830 - 0x7030) >> 2,
> > };
> >
> > @@ -1359,13 +1360,13 @@ static void dce_v6_0_audio_enable(struct
> > amdgpu_device *adev,
> >
> > static const u32 pin_offsets[7] =
> > {
> > - (0x1780 - 0x1780),
> > - (0x1786 - 0x1780),
> > - (0x178c - 0x1780),
> > - (0x1792 - 0x1780),
> > - (0x1798 - 0x1780),
> > - (0x179d - 0x1780),
> > - (0x17a4 - 0x1780),
> > + AUD0_REGISTER_OFFSET,
> > + AUD1_REGISTER_OFFSET,
> > + AUD2_REGISTER_OFFSET,
> > + AUD3_REGISTER_OFFSET,
> > + AUD4_REGISTER_OFFSET,
> > + AUD5_REGISTER_OFFSET,
> > + AUD6_REGISTER_OFFSET,
> > };
> >
> > static int dce_v6_0_audio_init(struct amdgpu_device *adev)
> > @@ -2876,22 +2877,22 @@ static void
> > dce_v6_0_set_crtc_vblank_interrupt_state(struct amdgpu_device *adev,
> >
> > switch (crtc) {
> > case 0:
> > - reg_block = SI_CRTC0_REGISTER_OFFSET;
> > + reg_block = CRTC0_REGISTER_OFFSET;
> > break;
> > case 1:
> > - reg_block = SI_CRTC1_REGISTER_OFFSET;
> > + reg_block = CRTC1_REGISTER_OFFSET;
> > break;
> > case 2:
> > - reg_block = SI_CRTC2_REGISTER_OFFSET;
> > + reg_block = CRTC2_REGISTER_OFFSET;
> > break;
> > case 3:
> > - reg_block = SI_CRTC3_REGISTER_OFFSET;
> > + reg_block = CRTC3_REGISTER_OFFSET;
> > break;
> > case 4:
> > - reg_block = SI_CRTC4_REGISTER_OFFSET;
> > + reg_block = CRTC4_REGISTER_OFFSET;
> > break;
> > case 5:
> > - reg_block = SI_CRTC5_REGISTER_OFFSET;
> > + reg_block = CRTC5_REGISTER_OFFSET;
> > break;
> > default:
> > DRM_DEBUG("invalid crtc %d\n", crtc);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/si_enums.h
> > b/drivers/gpu/drm/amd/amdgpu/si_enums.h
> > index 4e935baa7b91..cdf8eebaa392 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/si_enums.h
> > +++ b/drivers/gpu/d
[RFC PATCH 3/7] drm/amd/display: use drm_edid_product_id for parsing EDID product info
Since [1], we can use drm_edid_product_id to get debug info from drm_edid instead of directly parsing EDID. Link: https://lore.kernel.org/dri-devel/cover.1712655867.git.jani.nik...@intel.com/ [1] Signed-off-by: Melissa Wen --- .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 7edc23267ee7..0e37039dead0 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -109,6 +109,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps( struct drm_device *dev = connector->dev; struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL; struct drm_edid *drm_edid; + struct drm_edid_product_id product_id; struct cea_sad *sads; int sad_count = -1; int sadb_count = -1; @@ -126,13 +127,13 @@ enum dc_edid_status dm_helpers_parse_edid_caps( if (!drm_edid_valid(drm_edid)) result = EDID_BAD_CHECKSUM; - edid_caps->manufacturer_id = (uint16_t) edid_buf->mfg_id[0] | - ((uint16_t) edid_buf->mfg_id[1])<<8; - edid_caps->product_id = (uint16_t) edid_buf->prod_code[0] | - ((uint16_t) edid_buf->prod_code[1])<<8; - edid_caps->serial_number = edid_buf->serial; - edid_caps->manufacture_week = edid_buf->mfg_week; - edid_caps->manufacture_year = edid_buf->mfg_year; + drm_edid_get_product_id(drm_edid, &product_id); + + edid_caps->manufacturer_id = le16_to_cpu(product_id.manufacturer_name); + edid_caps->product_id = le16_to_cpu(product_id.product_code); + edid_caps->serial_number = le32_to_cpu(product_id.serial_number); + edid_caps->manufacture_week = product_id.week_of_manufacture; + edid_caps->manufacture_year = product_id.year_of_manufacture; drm_edid_get_monitor_name(edid_buf, edid_caps->display_name, -- 2.47.2
Re: [PATCH v2] drm/amdgpu: handle amdgpu_cgs_create_device() errors in amd_powerplay_create()
Applied. Thanks
On Thu, Mar 6, 2025 at 2:52 AM Wentao Liang wrote:
>
> Add error handling to propagate amdgpu_cgs_create_device() failures
> to the caller. When amdgpu_cgs_create_device() fails, release hwmgr
> and return -ENOMEM to prevent null pointer dereference.
>
> [v1]->[v2]: Change error code from -EINVAL to -ENOMEM. Free hwmgr.
>
> Signed-off-by: Wentao Liang
> ---
> drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> index 26624a716fc6..f8434158a402 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> @@ -51,6 +51,11 @@ static int amd_powerplay_create(struct amdgpu_device *adev)
> hwmgr->adev = adev;
> hwmgr->not_vf = !amdgpu_sriov_vf(adev);
> hwmgr->device = amdgpu_cgs_create_device(adev);
> + if (!hwmgr->device) {
> + kfree(hwmgr);
> + return -ENOMEM;
> + }
> +
> mutex_init(&hwmgr->msg_lock);
> hwmgr->chip_family = adev->family;
> hwmgr->chip_id = adev->asic_type;
> --
> 2.42.0.windows.2
>
Re: [PATCH] drm: amdkfd: Replace (un)register_chrdev() by (unregister/alloc)_chrdev_region()
On Wed, Mar 05, 2025 at 07:18:33PM -0500, Felix Kuehling wrote: > > On 2025-03-05 16:08, Salah Triki wrote: > > Replace (un)register_chrdev() by (unregister/alloc)_chrdev_region() as > > they are deprecated since kernel 2.6. > > Where is that information coming from? I see __register_chrdev documented in > the current kernel documentation. I see no indication that it's deprecated: > https://docs.kernel.org/core-api/kernel-api.html#c.__register_chrdev > In the book "Linux Device Drivers" 3ed of J. Corbet and al. (2005), it is indicated that using register_chrdev() is the old way to register char drivers, the new code should not use this interface, it should instead use the cdev interface. > > > alloc_chrdev_region() generates a > > dev_t value, so replace the kfd_char_dev_major int variable by the > > kfd_char_dev_id dev_t variable and drop the MKDEV() call. Initialize a > > cdev structure and add it to the device driver model as register_chrdev() > > used to do and since alloc_chrdev_region() does not do it. Drop the > > iminor() call since alloc_chrdev_region() allocates only one minor number. > > On error and in the module exit function, remove the cdev structure from > > the device driver model as unregister_chrdev() used to do. > > Sounds complicated. Your patch seems to open-code a bunch of details that > are neatly hidden inside register_chrdev. Why would I want all that detail > in my driver? I don't see an obvious advantage. > register_chrdev() registers 256 minor numbers, calling it will result in calling kmalloc_array(256, sizeof(struct probe), GFP_KERNEL) whereas calling alloc_chrdev_region() with count parameter equals to 1, which is the number of minor numbers requested, will result in calling kmalloc_array(1, sizeof(stuct probe), GFP_KERNEL). Best Regards, Salah Triki
RE: [PATCH v2] drm/amdgpu: Use unique CPER record id across devices
[AMD Official Use Only - AMD Internal Distribution Only]
Reviewed-by: Tao Zhou
> -Original Message-
> From: Liu, Xiang(Dean)
> Sent: Thursday, March 6, 2025 4:23 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking ; Zhou1, Tao
> ; Liu, Xiang(Dean)
> Subject: [PATCH v2] drm/amdgpu: Use unique CPER record id across devices
>
> Encode socket id to CPER record id to be unique across devices.
>
> v2: add pointer check for adev->smuio.funcs->get_socket_id
> v2: set 0 if adev->smuio.funcs->get_socket_id is NULL
>
> Signed-off-by: Xiang Liu
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
> index d77da7ca9a87..bc8473306063 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
> @@ -57,6 +57,8 @@ void amdgpu_cper_entry_fill_hdr(struct amdgpu_device *adev,
> enum amdgpu_cper_type type,
> enum cper_error_severity sev)
> {
> + char record_id[16];
> +
> hdr->signature[0] = 'C';
> hdr->signature[1] = 'P';
> hdr->signature[2] = 'E';
> @@ -71,7 +73,13 @@ void amdgpu_cper_entry_fill_hdr(struct amdgpu_device
> *adev,
>
> amdgpu_cper_get_timestamp(&hdr->timestamp);
>
> - snprintf(hdr->record_id, 8, "%d",
> atomic_inc_return(&adev->cper.unique_id));
> + snprintf(record_id, 9, "%d:%X",
> + (adev->smuio.funcs && adev->smuio.funcs->get_socket_id) ?
> + adev->smuio.funcs->get_socket_id(adev) :
> + 0,
> + atomic_inc_return(&adev->cper.unique_id));
> + memcpy(hdr->record_id, record_id, 8);
> +
> snprintf(hdr->platform_id, 16, "0x%04X:0x%04X",
>adev->pdev->vendor, adev->pdev->device);
> /* pmfw version should be part of creator_id according to CPER spec */
> @@ -
> 117,10 +125,10 @@ static int amdgpu_cper_entry_fill_section_desc(struct
> amdgpu_device *adev,
> section_desc->severity = sev;
> section_desc->sec_type = sec_type;
>
> - if (adev->smuio.funcs &&
> - adev->smuio.funcs->get_socket_id)
> - snprintf(section_desc->fru_text, 20, "OAM%d",
> - adev->smuio.funcs->get_socket_id(adev));
> + snprintf(section_desc->fru_text, 20, "OAM%d",
> + (adev->smuio.funcs && adev->smuio.funcs->get_socket_id) ?
> + adev->smuio.funcs->get_socket_id(adev) :
> + 0);
>
> if (bp_threshold)
> section_desc->flag_bits.exceed_err_threshold = 1;
> --
> 2.34.1
[RFC PATCH 7/7] drm/amd/display: get SADB from drm_eld when parsing EDID caps
drm_edid_connector_update() updates display info, filling ELD with
speaker allocation data in the last step of update_dislay_info(). Our
goal is stopping using raw edid, so we can extract SADB from drm_eld
instead of access raw edid to get audio caps.
Signed-off-by: Melissa Wen
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index a84363ad3c9a..f7f592fa98da 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -106,13 +106,10 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
struct drm_edid *drm_edid;
struct drm_edid_product_id product_id;
- int sad_count, sadb_count;
+ int sad_count;
int i = 0;
- uint8_t *sadb = NULL;
-
enum dc_edid_status result = EDID_OK;
-
if (!edid_caps || !edid)
return EDID_BAD_INPUT;
@@ -158,19 +155,11 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
edid_caps->audio_modes[i].sample_size = sad.byte2;
}
- sadb_count = drm_edid_to_speaker_allocation((struct edid *)
edid->raw_edid, &sadb);
-
- if (sadb_count < 0) {
- DRM_ERROR("Couldn't read Speaker Allocation Data Block: %d\n",
sadb_count);
- sadb_count = 0;
- }
-
- if (sadb_count)
- edid_caps->speaker_flags = sadb[0];
+ if (connector->eld[DRM_ELD_SPEAKER])
+ edid_caps->speaker_flags = connector->eld[DRM_ELD_SPEAKER];
else
edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
- kfree(sadb);
drm_edid_free(drm_edid);
return result;
--
2.47.2
[RFC PATCH 4/7] drm/amd/display: parse display name from drm_eld
We don't need to parse dc_edid to get the display name since it's already set in drm_eld which in turn had it values updated when updating connector with the opaque drm_edid. Signed-off-by: Melissa Wen --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 0e37039dead0..0def4f59b05a 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -34,7 +34,7 @@ #include #include #include - +#include #include "dm_services.h" #include "amdgpu.h" #include "dc.h" @@ -90,6 +90,7 @@ static void apply_edid_quirks(struct drm_device *dev, struct edid *edid, struct } } +#define AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS 13 /** * dm_helpers_parse_edid_caps() - Parse edid caps * @@ -135,9 +136,10 @@ enum dc_edid_status dm_helpers_parse_edid_caps( edid_caps->manufacture_week = product_id.week_of_manufacture; edid_caps->manufacture_year = product_id.year_of_manufacture; - drm_edid_get_monitor_name(edid_buf, - edid_caps->display_name, - AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS); + memset(edid_caps->display_name, 0, AUDIO_INFO_DISPLAY_NAME_SIZE_IN_CHARS); + memcpy(edid_caps->display_name, + &connector->eld[DRM_ELD_MONITOR_NAME_STRING], + AMDGPU_ELD_DISPLAY_NAME_SIZE_IN_CHARS); edid_caps->edid_hdmi = connector->display_info.is_hdmi; -- 2.47.2
[RFC PATCH 5/7] drm/amd/display: get panel id with drm_edid helper
Instead of using driver-specific code, use DRM helpers.
Signed-off-by: Melissa Wen
---
.../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 0def4f59b05a..e7a4513e7de2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -48,16 +48,11 @@
#include "ddc_service_types.h"
#include "clk_mgr.h"
-static u32 edid_extract_panel_id(struct edid *edid)
+static void apply_edid_quirks(struct drm_device *dev,
+ const struct drm_edid *drm_edid,
+ struct dc_edid_caps *edid_caps)
{
- return (u32)edid->mfg_id[0] << 24 |
- (u32)edid->mfg_id[1] << 16 |
- (u32)EDID_PRODUCT_ID(edid);
-}
-
-static void apply_edid_quirks(struct drm_device *dev, struct edid *edid,
struct dc_edid_caps *edid_caps)
-{
- uint32_t panel_id = edid_extract_panel_id(edid);
+ uint32_t panel_id = drm_edid_get_panel_id(drm_edid);
switch (panel_id) {
/* Workaround for monitors that need a delay after detecting the link */
@@ -143,7 +138,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
edid_caps->edid_hdmi = connector->display_info.is_hdmi;
- apply_edid_quirks(dev, edid_buf, edid_caps);
+ apply_edid_quirks(dev, drm_edid, edid_caps);
sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
if (sad_count <= 0) {
--
2.47.2
Re: [PATCH 04/11] drm/amdgpu/mes: centralize gfx_hqd mask management
On 3/6/2025 2:17 AM, Alex Deucher wrote:
Move it to amdgpu_mes to align with the compute and
sdma hqd masks. No functional change.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 24
drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 16 +++-
drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 15 +++
3 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index ca076306adba4..afc2ce344df52 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -144,6 +144,30 @@ int amdgpu_mes_init(struct amdgpu_device *adev)
adev->mes.vmid_mask_mmhub = 0xff00;
adev->mes.vmid_mask_gfxhub = 0xff00;
+ if (adev->gfx.num_gfx_rings) {
when kernel queue is disabled then arent we having gfx.num_gfx_rings ==
0, so this might not run at all ? Hope we taking care of that situation
too ?
+ for (i = 0; i < AMDGPU_MES_MAX_GFX_PIPES; i++) {
+ /* use only 1st ME pipe */
+ if (i >= adev->gfx.me.num_pipe_per_me)
+ continue;
this if condition makes the outside for loop to run just once and due to
which adev->mes.gfx_hqd_mask[1] = 0x0; is never set but based on
previous code we need to set that to 0
as pipe 1 is disabled in hq or here we do not need to set
adev->mes.gfx_hqd_mask at all now ?
+ if (amdgpu_ip_version(adev, GC_HWIP, 0) >=
+ IP_VERSION(12, 0, 0))
+ /*
+* GFX V12 has only one GFX pipe, but 8 queues
in it.
+* GFX pipe 0 queue 0 is being used by Kernel
queue.
+* Set GFX pipe 0 queue 1-7 for MES scheduling
+* mask = 1110b
+*/
+ adev->mes.gfx_hqd_mask[i] = 0xFE;
+ else
+ /*
+* GFX pipe 0 queue 0 is being used by Kernel
queue.
+* Set GFX pipe 0 queue 1 for MES scheduling
+* mask = 10b
+*/
+ adev->mes.gfx_hqd_mask[i] = 0x2;
+ }
+ }
+
for (i = 0; i < AMDGPU_MES_MAX_COMPUTE_PIPES; i++) {
/* use only 1st MEC pipes */
if (i >= adev->gfx.mec.num_pipe_per_mec)
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index a569d09a1a748..39b45d8b5f049 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -669,18 +669,6 @@ static int mes_v11_0_misc_op(struct amdgpu_mes *mes,
offsetof(union MESAPI__MISC, api_status));
}
-static void mes_v11_0_set_gfx_hqd_mask(union MESAPI_SET_HW_RESOURCES *pkt)
-{
- /*
-* GFX pipe 0 queue 0 is being used by Kernel queue.
-* Set GFX pipe 0 queue 1 for MES scheduling
-* mask = 10b
-* GFX pipe 1 can't be used for MES due to HW limitation.
-*/
- pkt->gfx_hqd_mask[0] = 0x2;
- pkt->gfx_hqd_mask[1] = 0;
-}
-
static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes)
{
int i;
@@ -705,7 +693,9 @@ static int mes_v11_0_set_hw_resources(struct amdgpu_mes
*mes)
mes_set_hw_res_pkt.compute_hqd_mask[i] =
mes->compute_hqd_mask[i];
- mes_v11_0_set_gfx_hqd_mask(&mes_set_hw_res_pkt);
+ for (i = 0; i < MAX_GFX_PIPES; i++)
+ mes_set_hw_res_pkt.gfx_hqd_mask[i] =
+ mes->gfx_hqd_mask[i];
for (i = 0; i < MAX_SDMA_PIPES; i++)
mes_set_hw_res_pkt.sdma_hqd_mask[i] = mes->sdma_hqd_mask[i];
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
index 96336652d14c5..519f054bec60d 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
@@ -694,17 +694,6 @@ static int mes_v12_0_set_hw_resources_1(struct amdgpu_mes
*mes, int pipe)
offsetof(union MESAPI_SET_HW_RESOURCES_1, api_status));
}
-static void mes_v12_0_set_gfx_hqd_mask(union MESAPI_SET_HW_RESOURCES *pkt)
-{
- /*
-* GFX V12 has only one GFX pipe, but 8 queues in it.
-* GFX pipe 0 queue 0 is being used by Kernel queue.
-* Set GFX pipe 0 queue 1-7 for MES scheduling
-* mask = 1110b
-*/
- pkt->gfx_hqd_mask[0] = 0xFE;
-}
-
static int mes_v12_0_set_hw_resources(struct amdgpu_mes *mes, int pipe)
{
int i;
@@ -727,7 +716,9 @@ static int mes_v12_0_set_hw_resources(struct amdgpu_mes
*mes, int pipe)
mes_set_hw_res_pkt.compute_hqd_mask[i] =
[RFC PATCH 2/7] drm/amd/display: start using drm_edid helpers to parse EDID caps
Groundwork that allocates a temporary drm_edid from raw edid to take
advantage of DRM common-code helpers instead of driver-specific code.
Signed-off-by: Melissa Wen
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 2cd35392e2da..7edc23267ee7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -108,6 +108,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
struct drm_connector *connector = &aconnector->base;
struct drm_device *dev = connector->dev;
struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
+ struct drm_edid *drm_edid;
struct cea_sad *sads;
int sad_count = -1;
int sadb_count = -1;
@@ -116,10 +117,13 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
enum dc_edid_status result = EDID_OK;
+
if (!edid_caps || !edid)
return EDID_BAD_INPUT;
- if (!drm_edid_is_valid(edid_buf))
+ drm_edid = drm_edid_alloc(edid_buf, EDID_LENGTH * (edid_buf->extensions
+ 1));
+
+ if (!drm_edid_valid(drm_edid))
result = EDID_BAD_CHECKSUM;
edid_caps->manufacturer_id = (uint16_t) edid_buf->mfg_id[0] |
@@ -139,8 +143,10 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
apply_edid_quirks(dev, edid_buf, edid_caps);
sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
- if (sad_count <= 0)
+ if (sad_count <= 0) {
+ drm_edid_free(drm_edid);
return result;
+ }
edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
for (i = 0; i < edid_caps->audio_mode_count; ++i) {
@@ -166,6 +172,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
kfree(sads);
kfree(sadb);
+ drm_edid_free(drm_edid);
return result;
}
--
2.47.2
[RFC PATCH 0/7] drm/amd/display: more DRM edid common-code to the display driver
Hi, I've been working on a new version of [1] that ports the AMD display driver to use the common `drm_edid` code instead of open and raw edid handling. The part of the previous series that didn't make the cut was to replace the open coded edid parser with `drm_edid_product_id` and `eld` data. However, when addressing feedback I ran into a design issue in the latest version because I was trying to not add any Linux-specific code to the DC code, specifically, DC link detection. In short, we have a few paths in the DM code that allocate a temporary `drm_edid`, go to the DC, and back to the DM to handle `drm_edid` data. In the last version, I was storing this temporary `drm_edid` in the aconnector, but it was erroneously overriding a still in use `drm_edid`. In this new version I allocate a temporary `drm_edid` in the DM parser from raw edid data stored in `dc_edid`, which was actually extracted from another `drm_edid` in the previous DM caller. This is a workaround to bypass DC boundaries and parse edid capabilities, but I think we can do better if we can find a clean way to pass the `drm_edid` through this kind of DM -> DC -> DM paths. I checked working on top of Thomas' work[2] that replaces `dc_edid` by `drm_edid` and adds this DRM struct and its helpers inside DC. The resulted changes work stable and as expected[3], but I believe that adding linux-specific code to DC is not desirable. Siqueira and I have discussed other alternatives, such as updating the DC code to match `drm_edid` structs or checking how well the change in this series could work with `drm_edid` as a private obj[4], however we would like to know AMD team's opinion before making this big effort (and probably noisy change). The main goal here is removing `drm_edid_raw` calls and duplicated code to handle edid in DC/link_detection that can take advantage of DRM common-code instead. Please, let me know your thoughts. Melissa [1] https://lore.kernel.org/amd-gfx/20240918213845.158293-1-mario.limoncie...@amd.com/ [2] https://lore.kernel.org/amd-gfx/20241112-amdgpu-drm_edid-v2-0-1399dc0f0...@weissschuh.net/ [3] https://gitlab.freedesktop.org/mwen/linux-amd/-/commits/drm_edid_product_id_v4 [4] https://gitlab.freedesktop.org/mwen/linux-amd/-/commits/drm_edid_priv Melissa Wen (7): drm/amd/display: make sure drm_edid stored in aconnector doesn't leak drm/amd/display: start using drm_edid helpers to parse EDID caps drm/amd/display: use drm_edid_product_id for parsing EDID product info drm/amd/display: parse display name from drm_eld drm/amd/display: get panel id with drm_edid helper drm/amd/display: get SAD from drm_eld when parsing EDID caps drm/amd/display: get SADB from drm_eld when parsing EDID caps .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 + .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 85 +-- 2 files changed, 42 insertions(+), 45 deletions(-) -- 2.47.2
[RFC PATCH 6/7] drm/amd/display: get SAD from drm_eld when parsing EDID caps
drm_edid_connector_update() updates display info, filling ELD with audio
info from Short-Audio Descriptors in the last step of
update_dislay_info(). Our goal is stopping using raw edid, so we can
extract SAD from drm_eld instead of access raw edid to get audio caps.
Signed-off-by: Melissa Wen
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 21 ++-
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index e7a4513e7de2..a84363ad3c9a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -106,9 +106,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
struct edid *edid_buf = edid ? (struct edid *) edid->raw_edid : NULL;
struct drm_edid *drm_edid;
struct drm_edid_product_id product_id;
- struct cea_sad *sads;
- int sad_count = -1;
- int sadb_count = -1;
+ int sad_count, sadb_count;
int i = 0;
uint8_t *sadb = NULL;
@@ -123,6 +121,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
if (!drm_edid_valid(drm_edid))
result = EDID_BAD_CHECKSUM;
+ drm_edid_connector_update(connector, drm_edid);
drm_edid_get_product_id(drm_edid, &product_id);
edid_caps->manufacturer_id = le16_to_cpu(product_id.manufacturer_name);
@@ -140,7 +139,7 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
apply_edid_quirks(dev, drm_edid, edid_caps);
- sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, &sads);
+ sad_count = drm_eld_sad_count(connector->eld);
if (sad_count <= 0) {
drm_edid_free(drm_edid);
return result;
@@ -148,12 +147,15 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
edid_caps->audio_mode_count = min(sad_count, DC_MAX_AUDIO_DESC_COUNT);
for (i = 0; i < edid_caps->audio_mode_count; ++i) {
- struct cea_sad *sad = &sads[i];
+ struct cea_sad sad;
- edid_caps->audio_modes[i].format_code = sad->format;
- edid_caps->audio_modes[i].channel_count = sad->channels + 1;
- edid_caps->audio_modes[i].sample_rate = sad->freq;
- edid_caps->audio_modes[i].sample_size = sad->byte2;
+ if (drm_eld_sad_get(connector->eld, i, &sad) < 0)
+ continue;
+
+ edid_caps->audio_modes[i].format_code = sad.format;
+ edid_caps->audio_modes[i].channel_count = sad.channels + 1;
+ edid_caps->audio_modes[i].sample_rate = sad.freq;
+ edid_caps->audio_modes[i].sample_size = sad.byte2;
}
sadb_count = drm_edid_to_speaker_allocation((struct edid *)
edid->raw_edid, &sadb);
@@ -168,7 +170,6 @@ enum dc_edid_status dm_helpers_parse_edid_caps(
else
edid_caps->speaker_flags = DEFAULT_SPEAKER_LOCATION;
- kfree(sads);
kfree(sadb);
drm_edid_free(drm_edid);
--
2.47.2
SI DMA: clarification on naming convention
Hi, While working on cleaning up sid.h, si_enums.h and some other SI related files, I've been scratching my head about why SI DMA files and variables were named "DMA" compared to CIK and over where "SDMA" is used. While I understand that a new system DMA was introduced with CIK, isn't SI DMA also a "system DMA"? Could we use the same naming convention and talk about sDMA, name defines values, shifts and masks? Could si_dma.c/.h be renamed to si_sdma.c/.h? Was the naming difference introduced to CIK so different that the naming covention needed to be modified? Thanks, Alexandre Demers
[RFC PATCH 1/7] drm/amd/display: make sure drm_edid stored in aconnector doesn't leak
Make sure the drm_edid container stored in aconnector is freed when
detroying the aconnector.
Fixes: 48edb2a4 ("drm/amd/display: switch amdgpu_dm_connector to use struct
drm_edid")
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
1 file changed, 2 insertions(+)
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 5939796db74c..dacb8b92c0d6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7271,6 +7271,8 @@ static void amdgpu_dm_connector_destroy(struct
drm_connector *connector)
dc_sink_release(aconnector->dc_sink);
aconnector->dc_sink = NULL;
+ drm_edid_free(aconnector->drm_edid);
+
drm_dp_cec_unregister_connector(&aconnector->dm_dp_aux.aux);
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
--
2.47.2
