Re: [PATCH] drm/amdgpu: clean up some inconsistent indenting
Am 01.09.23 um 09:02 schrieb Jiapeng Chong: No functional modification involved. drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c:34 nbio_v7_11_get_rev_id() warn: inconsistent indenting. We should probably not have a printk here in the first place. Christian. Reported-by: Abaci Robot Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=6316 Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c index 7c08e5f95e97..76e21357dd4d 100644 --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c @@ -31,10 +31,9 @@ static u32 nbio_v7_11_get_rev_id(struct amdgpu_device *adev) { u32 tmp; - printk("%s, getid\n",__func__); - - tmp = RREG32_SOC15(NBIO, 0, regRCC_STRAP1_RCC_DEV0_EPF0_STRAP0); + printk("%s, getid\n", __func__); + tmp = RREG32_SOC15(NBIO, 0, regRCC_STRAP1_RCC_DEV0_EPF0_STRAP0); tmp &= RCC_STRAP0_RCC_DEV0_EPF0_STRAP0__STRAP_ATI_REV_ID_DEV0_F0_MASK; tmp >>= RCC_STRAP0_RCC_DEV0_EPF0_STRAP0__STRAP_ATI_REV_ID_DEV0_F0__SHIFT;
Re: [Intel-gfx] [PATCH 0/4] drm/amd/display: stop using drm_edid_override_connector_update()
On Thu, Aug 31, 2023 at 6:01 PM Alex Hung wrote: > > > > On 2023-08-30 01:29, Jani Nikula wrote: > > On Tue, 29 Aug 2023, Alex Hung wrote: > >> On 2023-08-29 11:03, Jani Nikula wrote: > >>> On Tue, 29 Aug 2023, Jani Nikula wrote: > On Tue, 29 Aug 2023, Alex Deucher wrote: > > On Tue, Aug 29, 2023 at 6:48 AM Jani Nikula > > wrote: > >> > >> On Wed, 23 Aug 2023, Jani Nikula wrote: > >>> On Tue, 22 Aug 2023, Alex Hung wrote: > On 2023-08-22 06:01, Jani Nikula wrote: > > Over the past years I've been trying to unify the override and > > firmware > > EDID handling as well as EDID property updates. It won't work if > > drivers > > do their own random things. > Let's check how to replace these references by appropriate ones or > fork > the function as reverting these patches causes regressions. > >>> > >>> I think the fundamental problem you have is conflating connector > >>> forcing > >>> with EDID override. They're orthogonal. The .force callback has no > >>> business basing the decisions on connector->edid_override. Force is > >>> force, override is override. > >>> > >>> The driver isn't even supposed to know or care if the EDID originates > >>> from the firmware loader or override EDID debugfs. drm_get_edid() will > >>> handle that for you transparently. It'll return the EDID, and you > >>> shouldn't look at connector->edid_blob_ptr either. Using that will > >>> make > >>> future work in drm_edid.c harder. > >>> > >>> You can't fix that with minor tweaks. I think you'll be better off > >>> starting from scratch. > >>> > >>> Also, connector->edid_override is debugfs. You actually can change the > >>> behaviour. If your userspace, whatever it is, has been written to > >>> assume > >>> connector forcing if EDID override is set, you *do* have to fix that, > >>> and set both. > >> > >> Any updates on fixing this, or shall we proceed with the reverts? > >> > >> There is a patch under internal reviews. It removes calls edid_override > >> and drm_edid_override_connector_update as intended in this patchset but > >> does not remove the functionality. > > > > While I am happy to hear there's progress, I'm somewhat baffled the > > review is internal. The commits that I suggested to revert were also > > only reviewed internally, as far as I can see... And that's kind of the > > problem. > > > > Upstream code should be reviewed in public. > > Hi Jani, > > All patches are sent for public reviews, the progress is summarized as > the followings: > > == internal == > > 1. a patch or patches are tested by CI. > 2. internal technical and IP reviews are performed to ensure no concerns > before patches are merged to internal branch. > > == public == > > 3. a regression test and IP reviews are performed by engineers before > sending to public mailing lists. > 4. the patchset is sent for public reviews ex. > https://patchwork.freedesktop.org/series/122498/ > 5. patches are merged to public repo. > This sort of thing is fine for unreleased chips or new IP prior public exposure, but for released hardware, you really need to do the reviews on the mailing lists. Alex > > > > > > BR, > > Jani. > > > > > >> > >> With the patch. both following git grep commands return nothing in > >> amd-staging-drm-next. > >> > >> $ git grep drm_edid_override_connector_update -- drivers/gpu/drm/amd > >> $ git grep edid_override -- drivers/gpu/drm/amd > >> > >> Best regards, > >> Alex Hung > >> > > > > What is the goal of the reverts? I don't disagree that we may be > > using the interfaces wrong, but reverting them will regess > > functionality in the driver. > > The commits are in v6.5-rc1, but not yet in a release. No user depends > on them yet. I'd strongly prefer them not reaching v6.5 final and users. > >>> > >>> Sorry for confusion here, that's obviously come and gone already. :( > >>> > The firmware EDID, override EDID, connector forcing, the EDID property, > etc. have been and somewhat still are a hairy mess that we must keep > untangling, and this isn't helping. > > I've put in crazy amounts of work on this, and I've added kernel-doc > comments about stuff that should and should not be done, but they go > unread and ignored. > > I really don't want to end up having to clean this up myself before I > can embark on further cleanups and refactoring. > > And again, if the functionality in the driver depends on conflating two > things that should be separate, it's probably not such a hot idea to let > it reach users either. Even if it's just debugfs. > > > BR, > Jani. > >>> > >
Re: [Intel-gfx] [PATCH 0/4] drm/amd/display: stop using drm_edid_override_connector_update()
On Thu, 31 Aug 2023, Alex Hung wrote: > On 2023-08-30 01:29, Jani Nikula wrote: >> On Tue, 29 Aug 2023, Alex Hung wrote: >>> There is a patch under internal reviews. It removes calls edid_override >>> and drm_edid_override_connector_update as intended in this patchset but >>> does not remove the functionality. >> >> While I am happy to hear there's progress, I'm somewhat baffled the >> review is internal. The commits that I suggested to revert were also >> only reviewed internally, as far as I can see... And that's kind of the >> problem. >> >> Upstream code should be reviewed in public. > > Hi Jani, > > All patches are sent for public reviews, the progress is summarized as > the followings: > > == internal == > > 1. a patch or patches are tested by CI. > 2. internal technical and IP reviews are performed to ensure no concerns > before patches are merged to internal branch. > > == public == > > 3. a regression test and IP reviews are performed by engineers before > sending to public mailing lists. > 4. the patchset is sent for public reviews ex. > https://patchwork.freedesktop.org/series/122498/ > 5. patches are merged to public repo. The point about public review is that there's no transparency to the steps before 4. The patches are posted for public review with Reviewed-by and Acked-by already added, based on internal review, and there is de facto no public review taking place on the code drops. There is zero visibility to the discussions taking place. We don't know if it's just rubber stamping, we don't know what concerns were raised, if any. I'm mainly disappointed about the double standards here, given that we post most patches directly upstream (especially ones that have zero reason to be embargoed like the ones being discussed here), and the ones that have gone through internal review will be stripped of all prior internal Reviewed-by's and Acked-by's before posting. Because that's the upstream expectation. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
[PATCH -next 3/5] drm/amd/display: clean up some inconsistent indentings
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_hwseq.c:159 dcn35_init_hw() warn: inconsistent indentig Signed-off-by: Yang Li --- .../drm/amd/display/dc/dcn35/dcn35_hwseq.c| 32 +-- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c index 666e2809d9dc..025849143254 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c @@ -155,22 +155,22 @@ void dcn35_init_hw(struct dc *dc) res_pool->ref_clocks.xtalin_clock_inKhz = dc->ctx->dc_bios->fw_info.pll_info.crystal_frequency; - if (res_pool->dccg && res_pool->hubbub) { - - (res_pool->dccg->funcs->get_dccg_ref_freq)(res_pool->dccg, - dc->ctx->dc_bios->fw_info.pll_info.crystal_frequency, - &res_pool->ref_clocks.dccg_ref_clock_inKhz); - - (res_pool->hubbub->funcs->get_dchub_ref_freq)(res_pool->hubbub, - res_pool->ref_clocks.dccg_ref_clock_inKhz, - &res_pool->ref_clocks.dchub_ref_clock_inKhz); - } else { - // Not all ASICs have DCCG sw component - res_pool->ref_clocks.dccg_ref_clock_inKhz = - res_pool->ref_clocks.xtalin_clock_inKhz; - res_pool->ref_clocks.dchub_ref_clock_inKhz = - res_pool->ref_clocks.xtalin_clock_inKhz; - } + if (res_pool->dccg && res_pool->hubbub) { + + (res_pool->dccg->funcs->get_dccg_ref_freq)(res_pool->dccg, + dc->ctx->dc_bios->fw_info.pll_info.crystal_frequency, + &res_pool->ref_clocks.dccg_ref_clock_inKhz); + + (res_pool->hubbub->funcs->get_dchub_ref_freq)(res_pool->hubbub, + res_pool->ref_clocks.dccg_ref_clock_inKhz, + &res_pool->ref_clocks.dchub_ref_clock_inKhz); + } else { + // Not all ASICs have DCCG sw component + res_pool->ref_clocks.dccg_ref_clock_inKhz = + res_pool->ref_clocks.xtalin_clock_inKhz; + res_pool->ref_clocks.dchub_ref_clock_inKhz = + res_pool->ref_clocks.xtalin_clock_inKhz; + } } else ASSERT_CRITICAL(false); -- 2.20.1.7.g153144c
[PATCH -next 1/4] drm/amd/display: Remove duplicated include in dcn35_resource.c
./drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c: dcn31/dcn31_dio_link_encoder.h is included more than once. Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c index 0386b8fb270d..7f059fc2fc75 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c @@ -61,7 +61,6 @@ #include "dcn32/dcn32_hpo_dp_link_encoder.h" #include "link.h" #include "dcn31/dcn31_apg.h" -#include "dcn31/dcn31_dio_link_encoder.h" #include "dcn32/dcn32_dio_link_encoder.h" #include "dcn31/dcn31_vpg.h" #include "dcn31/dcn31_afmt.h" -- 2.20.1.7.g153144c
[PATCH -next 5/5] drm/amd/display: clean up one inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn35/dcn35_fpu.c:260 dcn35_update_bw_bounding_box_fpu() warn: inconsistent indenting Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c index 525ca0ed9ea9..46eb2d0592f3 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn35/dcn35_fpu.c @@ -348,8 +348,8 @@ void dcn35_update_bw_bounding_box_fpu(struct dc *dc, dc->debug.dram_clock_change_latency_ns / 1000.0; } /*temp till dml2 fully work without dml1*/ - dml_init_instance(&dc->dml, &dcn3_5_soc, &dcn3_5_ip, - DML_PROJECT_DCN31); + dml_init_instance(&dc->dml, &dcn3_5_soc, &dcn3_5_ip, + DML_PROJECT_DCN31); } static bool is_dual_plane(enum surface_pixel_format format) -- 2.20.1.7.g153144c
[PATCH -next 1/5] drm/amd/display: clean up one inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_srv.c:355 dmub_srv_hw_setup() warn: inconsistent indenting Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c b/drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c index 762549753721..b99db771e071 100644 --- a/drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c +++ b/drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c @@ -352,7 +352,7 @@ static bool dmub_srv_hw_setup(struct dmub_srv *dmub, enum dmub_asic asic) funcs->init_reg_offsets = dmub_srv_dcn35_regs_init; funcs->is_hw_powered_up = dmub_dcn35_is_hw_powered_up; - break; + break; default: return false; -- 2.20.1.7.g153144c
[PATCH -next 3/4] drm/amd/display: Remove duplicated include in dcn35_hwseq.c
./drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c: clk_mgr.h is included more than once. Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c index cacb557a3014..666e2809d9dc 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_hwseq.c @@ -31,7 +31,6 @@ #include "clk_mgr.h" #include "reg_helper.h" #include "abm.h" -#include "clk_mgr.h" #include "hubp.h" #include "dchubbub.h" #include "timing_generator.h" -- 2.20.1.7.g153144c
[PATCH] drm/amd: Make fence wait in suballocator uninterruptible
c103a23f2f297c6ab2e5e74e39b655439f3524a6 made the fence wait in amdgpu_sa_bo_new() interruptible but there is no code to handle an interrupt. This caused the kernel to randomly explode in high-VRAM-pressure situations so make it uninterruptible again. Signed-off-by: Simon Pilkington --- drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c index c6b4337eb20c..10df731998b2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c @@ -81,7 +81,7 @@ int amdgpu_sa_bo_new(struct amdgpu_sa_manager *sa_manager, unsigned int size) { struct drm_suballoc *sa = drm_suballoc_new(&sa_manager->base, size, - GFP_KERNEL, true, 0); + GFP_KERNEL, false, 0); if (IS_ERR(sa)) { *sa_bo = NULL; -- 2.40.1
Re: [V11 3/8] wifi: mac80211: Add support for WBRF features
On 8/30/2023 11:20 PM, Evan Quan wrote: To support the WBRF mechanism, Wifi adapters utilized in the system must Since this is the first mention of WBRF in the core wireless code IMO you should indicate what this is an acronym for and briefly describe it (or add a lore link). I'm wondering if WBRF is just a special case of frequency avoidance, and that more generic naming/terminology should be used in core wireless. For example, I know there are vendor-specific solutions which allow Wi-Fi to avoid using channels which may conflict with cellular or BlueTooth, and those may benefit from a more generic register the frequencies in use(or unregister those frequencies no longer used) via the dedicated calls. So that, other drivers responding to the frequencies can take proper actions to mitigate possible interference. Co-developed-by: Mario Limonciello Signed-off-by: Mario Limonciello Co-developed-by: Evan Quan Signed-off-by: Evan Quan -- v1->v2: - place the new added member(`wbrf_supported`) in ieee80211_local(Johannes) - handle chandefs change scenario properly(Johannes) - some minor fixes around code sharing and possible invalid input checks(Johannes) v2->v3: - drop unnecessary input checks and intermediate APIs(Mario) - Separate some mac80211 common code(Mario, Johannes) v3->v4: - some minor fixes around return values(Johannes) v9->v10: - get ranges_in->num_of_ranges set and passed in(Johannes) --- include/linux/ieee80211.h | 1 + net/mac80211/Makefile | 2 + net/mac80211/chan.c| 9 net/mac80211/ieee80211_i.h | 9 net/mac80211/main.c| 2 + net/mac80211/wbrf.c| 105 + 6 files changed, 128 insertions(+) create mode 100644 net/mac80211/wbrf.c diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h index 4b998090898e..f995d06da87f 100644 --- a/include/linux/ieee80211.h +++ b/include/linux/ieee80211.h @@ -4335,6 +4335,7 @@ static inline int ieee80211_get_tdls_action(struct sk_buff *skb, u32 hdr_size) /* convert frequencies */ #define MHZ_TO_KHZ(freq) ((freq) * 1000) #define KHZ_TO_MHZ(freq) ((freq) / 1000) +#define KHZ_TO_HZ(freq) ((freq) * 1000) #define PR_KHZ(f) KHZ_TO_MHZ(f), f % 1000 #define KHZ_F "%d.%03d" diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile index b8de44da1fb8..d46c36f55fd3 100644 --- a/net/mac80211/Makefile +++ b/net/mac80211/Makefile @@ -65,4 +65,6 @@ rc80211_minstrel-$(CONFIG_MAC80211_DEBUGFS) += \ mac80211-$(CONFIG_MAC80211_RC_MINSTREL) += $(rc80211_minstrel-y) +mac80211-y += wbrf.o + ccflags-y += -DDEBUG diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c index 68952752b599..458469c224ae 100644 --- a/net/mac80211/chan.c +++ b/net/mac80211/chan.c @@ -506,11 +506,16 @@ static void _ieee80211_change_chanctx(struct ieee80211_local *local, WARN_ON(!cfg80211_chandef_compatible(&ctx->conf.def, chandef)); + ieee80211_remove_wbrf(local, &ctx->conf.def); + ctx->conf.def = *chandef; /* check if min chanctx also changed */ changed = IEEE80211_CHANCTX_CHANGE_WIDTH | _ieee80211_recalc_chanctx_min_def(local, ctx, rsvd_for); + + ieee80211_add_wbrf(local, &ctx->conf.def); + drv_change_chanctx(local, ctx, changed); if (!local->use_chanctx) { @@ -668,6 +673,8 @@ static int ieee80211_add_chanctx(struct ieee80211_local *local, lockdep_assert_held(&local->mtx); lockdep_assert_held(&local->chanctx_mtx); + ieee80211_add_wbrf(local, &ctx->conf.def); + if (!local->use_chanctx) local->hw.conf.radar_enabled = ctx->conf.radar_enabled; @@ -748,6 +755,8 @@ static void ieee80211_del_chanctx(struct ieee80211_local *local, } ieee80211_recalc_idle(local); + + ieee80211_remove_wbrf(local, &ctx->conf.def); } static void ieee80211_free_chanctx(struct ieee80211_local *local, diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 91633a0b723e..719f2c892132 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1600,6 +1600,8 @@ struct ieee80211_local { /* extended capabilities provided by mac80211 */ u8 ext_capa[8]; + + bool wbrf_supported; }; static inline struct ieee80211_sub_if_data * @@ -2638,4 +2640,11 @@ ieee80211_eht_cap_ie_to_sta_eht_cap(struct ieee80211_sub_if_data *sdata, const struct ieee80211_eht_cap_elem *eht_cap_ie_elem, u8 eht_cap_len, struct link_sta_info *link_sta); + +void ieee80211_check_wbrf_support(struct ieee80211_local *local); +void ieee80211_add_wbrf(struct ieee80211_local *local, + struct cfg80211_chan_def *chandef); +void ieee80211_remove_wbrf(struct ieee80211_local *local, + struct cfg80211_chan_def *chandef); + #endif /* IEEE80211_I_H
[PATCH -next 2/5] drm/amd/display: clean up one inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_resource.c:1877 dcn35_resource_construct() warn: inconsistent indenting Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c index 7f059fc2fc75..bba747667a73 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_resource.c @@ -1873,7 +1873,7 @@ static bool dcn35_resource_construct( } } /*temp till dml2 fully work without dml1*/ - dml_init_instance(&dc->dml, &dcn3_5_soc, &dcn3_5_ip, DML_PROJECT_DCN31); + dml_init_instance(&dc->dml, &dcn3_5_soc, &dcn3_5_ip, DML_PROJECT_DCN31); /* TODO: DCCG */ pool->base.dccg = dccg35_create(ctx, &dccg_regs, &dccg_shift, &dccg_mask); -- 2.20.1.7.g153144c
[PATCH] drm/amdgpu: clean up some inconsistent indenting
No functional modification involved. drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c:34 nbio_v7_11_get_rev_id() warn: inconsistent indenting. Reported-by: Abaci Robot Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=6316 Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c index 7c08e5f95e97..76e21357dd4d 100644 --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c @@ -31,10 +31,9 @@ static u32 nbio_v7_11_get_rev_id(struct amdgpu_device *adev) { u32 tmp; - printk("%s, getid\n",__func__); - - tmp = RREG32_SOC15(NBIO, 0, regRCC_STRAP1_RCC_DEV0_EPF0_STRAP0); + printk("%s, getid\n", __func__); + tmp = RREG32_SOC15(NBIO, 0, regRCC_STRAP1_RCC_DEV0_EPF0_STRAP0); tmp &= RCC_STRAP0_RCC_DEV0_EPF0_STRAP0__STRAP_ATI_REV_ID_DEV0_F0_MASK; tmp >>= RCC_STRAP0_RCC_DEV0_EPF0_STRAP0__STRAP_ATI_REV_ID_DEV0_F0__SHIFT; -- 2.20.1.7.g153144c
[PATCH -next 4/5] drm/amd/display: clean up some inconsistent indentings
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c:288 dcn35_update_clocks() warn: inconsistent indenting Signed-off-by: Yang Li --- .../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c | 28 +-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c index 9314e75195cd..98d6a1f8af60 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c @@ -288,8 +288,8 @@ void dcn35_update_clocks(struct clk_mgr *clk_mgr_base, } // workaround: Limit dppclk to 100Mhz to avoid lower eDP panel switch to plus 4K monitor underflow. - if (new_clocks->dppclk_khz < 10) - new_clocks->dppclk_khz = 10; + if (new_clocks->dppclk_khz < 10) + new_clocks->dppclk_khz = 10; if (should_set_clock(safe_to_lower, new_clocks->dppclk_khz, clk_mgr->base.clks.dppclk_khz)) { if (clk_mgr->base.clks.dppclk_khz > new_clocks->dppclk_khz) @@ -901,21 +901,21 @@ void dcn35_clk_mgr_construct( ASSERT(smu_dpm_clks.dpm_clks); - clk_mgr->base.smu_ver = dcn35_smu_get_smu_version(&clk_mgr->base); + clk_mgr->base.smu_ver = dcn35_smu_get_smu_version(&clk_mgr->base); - if (clk_mgr->base.smu_ver) - clk_mgr->base.smu_present = true; + if (clk_mgr->base.smu_ver) + clk_mgr->base.smu_present = true; - /* TODO: Check we get what we expect during bringup */ - clk_mgr->base.base.dentist_vco_freq_khz = get_vco_frequency_from_reg(&clk_mgr->base); + /* TODO: Check we get what we expect during bringup */ + clk_mgr->base.base.dentist_vco_freq_khz = get_vco_frequency_from_reg(&clk_mgr->base); - if (ctx->dc_bios->integrated_info->memory_type == LpDdr5MemType) { - dcn35_bw_params.wm_table = lpddr5_wm_table; - } else { - dcn35_bw_params.wm_table = ddr5_wm_table; - } - /* Saved clocks configured at boot for debug purposes */ -dcn35_dump_clk_registers(&clk_mgr->base.base.boot_snapshot, &clk_mgr->base.base, &log_info); + if (ctx->dc_bios->integrated_info->memory_type == LpDdr5MemType) { + dcn35_bw_params.wm_table = lpddr5_wm_table; + } else { + dcn35_bw_params.wm_table = ddr5_wm_table; + } + /* Saved clocks configured at boot for debug purposes */ + dcn35_dump_clk_registers(&clk_mgr->base.base.boot_snapshot, &clk_mgr->base.base, &log_info); clk_mgr->base.base.dprefclk_khz = dcn35_smu_get_dprefclk(&clk_mgr->base); clk_mgr->base.base.clks.ref_dtbclk_khz = dcn35_smu_get_dtbclk(&clk_mgr->base); -- 2.20.1.7.g153144c
[PATCH -next 4/4] drm/amd/display: Remove duplicated include in dcn35_clk_mgr.c
./drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c: dcn35_clk_mgr.h is included more than once. Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c index 3b2463c03694..9314e75195cd 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c @@ -46,7 +46,6 @@ /* TODO: remove this include once we ported over remaining clk mgr functions*/ #include "dcn30/dcn30_clk_mgr.h" #include "dcn31/dcn31_clk_mgr.h" -#include "dcn35_clk_mgr.h" #include "dc_dmub_srv.h" #include "link.h" -- 2.20.1.7.g153144c
[PATCH] drm/amd: Make fence wait in suballocator uninterruptible
Commit c103a23f2f29 ("drm/amd: Convert amdgpu to use suballocation helper.") made the fence wait in amdgpu_sa_bo_new() interruptible but there is no code to handle an interrupt. This caused the kernel to randomly explode in high-VRAM-pressure situations so make it uninterruptible again. Fixes: c103a23f2f29 ("drm/amd: Convert amdgpu to use suballocation helper.") Signed-off-by: Simon Pilkington --- drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c index c6b4337eb20c..10df731998b2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c @@ -81,7 +81,7 @@ int amdgpu_sa_bo_new(struct amdgpu_sa_manager *sa_manager, unsigned int size) { struct drm_suballoc *sa = drm_suballoc_new(&sa_manager->base, size, - GFP_KERNEL, true, 0); + GFP_KERNEL, false, 0); if (IS_ERR(sa)) { *sa_bo = NULL; -- 2.40.1
[PATCH -next 2/4] drm/amd/display: Remove duplicated include in dcn35_optc.c
./drivers/gpu/drm/amd/display/dc/dcn35/dcn35_optc.c: dcn35_optc.h is included more than once. Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/dcn35/dcn35_optc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_optc.c b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_optc.c index d64be1a5071c..2bea1e475096 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_optc.c +++ b/drivers/gpu/drm/amd/display/dc/dcn35/dcn35_optc.c @@ -23,7 +23,6 @@ */ #include "dcn35_optc.h" -#include "dcn35_optc.h" #include "dcn30/dcn30_optc.h" #include "dcn31/dcn31_optc.h" -- 2.20.1.7.g153144c
Re: [Patch v2 2/3] drm/mst: Refactor the flow for payload allocation/removement
On Thu, Aug 31, 2023 at 6:45 PM Lyude Paul wrote: > > On Thu, 2023-08-24 at 04:12 +, Lin, Wayne wrote: > > [Public] > > > > Hi Lyude, > > > > I'm afraid that I don't have the permissions to push and would like to have > > your help. Thanks! > > Whoops, sorry I only just noticed this message. I set a reminder on my phone > to bug me to push it tomorrow :), sorry about the delay > Wayne, I don't see why we couldn't give you commit permissions for drm-misc. Alex > > > > > -Original Message- > > > From: Lyude Paul > > > Sent: Thursday, August 24, 2023 5:00 AM > > > To: Lin, Wayne ; dri-de...@lists.freedesktop.org; > > > amd-gfx@lists.freedesktop.org > > > Cc: jani.nik...@intel.com; ville.syrj...@linux.intel.com; > > > imre.d...@intel.com; > > > Wentland, Harry ; Zuo, Jerry > > > > > > Subject: Re: [Patch v2 2/3] drm/mst: Refactor the flow for payload > > > allocation/removement > > > > > > Sure - you're also welcome to push the first two patches after fixing the > > > indentation if you'd like > > > > > > On Wed, 2023-08-23 at 03:19 +, Lin, Wayne wrote: > > > > [Public] > > > > > > > > Thanks, Lyude! > > > > Should I push another version to fix the indention? > > > > > > > > > -Original Message- > > > > > From: Lyude Paul > > > > > Sent: Friday, August 18, 2023 6:17 AM > > > > > To: Lin, Wayne ; dri-de...@lists.freedesktop.org; > > > > > amd-gfx@lists.freedesktop.org > > > > > Cc: jani.nik...@intel.com; ville.syrj...@linux.intel.com; > > > > > imre.d...@intel.com; Wentland, Harry ; Zuo, > > > > > Jerry > > > > > Subject: Re: [Patch v2 2/3] drm/mst: Refactor the flow for payload > > > > > allocation/removement > > > > > > > > > > Two small comments: > > > > > > > > > > On Mon, 2023-08-07 at 10:56 +0800, Wayne Lin wrote: > > > > > > [Why] > > > > > > Today, the allocation/deallocation steps and status is a bit > > > > > > unclear. > > > > > > > > > > > > For instance, payload->vc_start_slot = -1 stands for "the failure > > > > > > of updating DPCD payload ID table" and can also represent as > > > > > > "payload is not allocated yet". These two cases should be handled > > > > > > differently and hence better to distinguish them for better > > > > > > understanding. > > > > > > > > > > > > [How] > > > > > > Define enumeration - ALLOCATION_LOCAL, ALLOCATION_DFP and > > > > > > ALLOCATION_REMOTE to distinguish different allocation status. > > > > > > Adjust the code to handle different status accordingly for better > > > > > > understanding the sequence of payload allocation and payload > > > > > removement. > > > > > > > > > > > > For payload creation, the procedure should look like this: > > > > > > DRM part 1: > > > > > > * step 1 - update sw mst mgr variables to add a new payload > > > > > > * step 2 - add payload at immediate DFP DPCD payload table > > > > > > > > > > > > Driver: > > > > > > * Add new payload in HW and sync up with DFP by sending ACT > > > > > > > > > > > > DRM Part 2: > > > > > > * Send ALLOCATE_PAYLOAD sideband message to allocate bandwidth > > > > > > along > > > > > the > > > > > > virtual channel. > > > > > > > > > > > > And as for payload removement, the procedure should look like this: > > > > > > DRM part 1: > > > > > > * step 1 - Send ALLOCATE_PAYLOAD sideband message to release > > > bandwidth > > > > > >along the virtual channel > > > > > > * step 2 - Clear payload allocation at immediate DFP DPCD payload > > > > > > table > > > > > > > > > > > > Driver: > > > > > > * Remove the payload in HW and sync up with DFP by sending ACT > > > > > > > > > > > > DRM part 2: > > > > > > * update sw mst mgr variables to remove the payload > > > > > > > > > > > > Note that it's fine to fail when communicate with the branch > > > > > > device connected at immediate downstrean-facing port, but updating > > > > > > variables of SW mst mgr and HW configuration should be conducted > > > > > > anyway. That's because it's under commit_tail and we need to > > > > > > complete the HW > > > > > programming. > > > > > > > > > > > > Changes since v1: > > > > > > * Remove the set but not use variable 'old_payload' in function > > > > > > 'nv50_msto_prepare'. Catched by kernel test robot > > > > > > > > > > > > > > > > > > Signed-off-by: Wayne Lin > > > > > > --- > > > > > > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 20 ++- > > > > > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 159 > > > > > > +++-- > > > > > - > > > > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 18 +- > > > > > > drivers/gpu/drm/nouveau/dispnv50/disp.c | 21 +-- > > > > > > include/drm/display/drm_dp_mst_helper.h | 23 ++- > > > > > > 5 files changed, 153 insertions(+), 88 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 d9a482908380..9ad509279b0a 100644 > > > > > > --- a/drivers/gpu/drm/
[RFT PATCH 00/15] drm: non-drm-misc drivers call drm_atomic_helper_shutdown() at the right times
NOTE: in order to avoid email sending limits on the cover letter, I've split this patch series in two. Patches that target drm-misc and ones that don't. The cover letter of the two is identical other than this note. This patch series came about after a _long_ discussion between me and Maxime Ripard in response to a different patch I sent out [1]. As part of that discussion, we realized that it would be good if DRM drivers consistently called drm_atomic_helper_shutdown() properly at shutdown and driver remove time as it's documented that they should do. The eventual goal of this would be to enable removing some hacky code from panel drivers where they had to hook into shutdown themselves because the DRM driver wasn't calling them. It turns out that quite a lot of drivers seemed to be missing drm_atomic_helper_shutdown() in one or both places that it was supposed to be. This patch series attempts to fix all the drivers that I was able to identify. NOTE: fixing this wasn't exactly cookie cutter. Each driver has its own unique way of setting itself up and tearing itself down. Some drivers also use the component model, which adds extra fun. I've made my best guess at solving this and I've run a bunch of compile tests (specifically, allmodconfig for amd64, arm64, and powerpc). That being said, these code changes are not totally trivial and I've done zero real testing on them. Making these patches was also a little mind numbing and I'm certain my eyes glazed over at several points when writing them. What I'm trying to say is to please double-check that I didn't do anything too silly, like cast your driver's drvdata to the wrong type. Even better, test these patches! I've organized this series like this: 1. One patch for all simple cases of just needing a call at shutdown time for drivers that go through drm-misc. 2. A separate patch for "drm/vc4", even though it goes through drm-misc, since I wanted to leave an extra note for that one. 3. Patches for drivers that just needed a call at shutdown time for drivers that _don't_ go through drm-misc. 4. Patches for the few drivers that had the call at shutdown time but lacked it at remove time. 5. One patch for all simple cases of needing a call at shutdown and remove (or unbind) time for drivers that go through drm-misc. 6. A separate patch for "drm/hisilicon/kirin", even though it goes through drm-misc, since I wanted to leave an extra note for that one. 7. Patches for drivers that needed a call at shutdown and remove (or unbind) time for drivers that _don't_ go through drm-misc. I've labeled this patch series as RFT (request for testing) to help call attention to the fact that I didn't personally test any of these patches. If you're a maintainer of one of these drivers and you think that the patch for your driver looks fabulous, you've tested it, and you'd like to land it right away then please do. For non-drm-misc drivers there are no dependencies here. Some of the drm-misc drivers depend on the first patch, AKA ("drm/atomic-helper: drm_atomic_helper_shutdown(NULL) should be a noop"). I've tried to call this out but it also should be obvious once you know to look for it. I'd like to call out a few drivers that I _didn't_ fix in this series and why. If any of these drivers should be fixed then please yell. - DRM driers backed by usb_driver (like gud, gm12u320, udl): I didn't add the call to drm_atomic_helper_shutdown() at shutdown time because there's no ".shutdown" callback for them USB drivers. Given that USB is hotpluggable, I'm assuming that they are robust against this and the special shutdown callback isn't needed. - ofdrm and simpledrm: These didn't have drm_atomic_helper_shutdown() in either shutdown or remove, but I didn't add it. I think that's OK since they're sorta special and not really directly controlling hardware power sequencing. - virtio, vkms, vmwgfx, xen: I believe these are all virtual (thus they wouldn't directly drive a panel) and adding the shutdown didn't look straightforward, so I skipped them. I've let each patch in the series get CCed straight from get_maintainer. That means not everyone will have received every patch but everyone should be on the cover letter. I know some people dislike this but when touching this many drivers there's not much choice. dri-devel and lkml have been CCed and lore/lei exist, so hopefully that's enough for folks. I'm happy to add people to the whole series for future posts. [1] https://lore.kernel.org/lkml/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid Douglas Anderson (15): drm/armada: Call drm_atomic_helper_shutdown() at shutdown time drm/imx/dcss: Call drm_atomic_helper_shutdown() at shutdown time drm/ingenic: Call drm_atomic_helper_shutdown() at shutdown time drm/kmb: Call drm_atomic_helper_shutdown() at shutdown time drm/mediatek: Call drm_atomic_helper_shutdown() at shutdown time drm/nouveau: Call drm_atomic_help
[RFT PATCH 09/15] drm/amdgpu: Call drm_atomic_helper_shutdown() at shutdown time
Based on grepping through the source code this driver appears to be missing a call to drm_atomic_helper_shutdown() at system shutdown time. Among other things, this means that if a panel is in use that it won't be cleanly powered off at system shutdown time. The fact that we should call drm_atomic_helper_shutdown() in the case of OS shutdown/restart comes straight out of the kernel doc "driver instance overview" in drm_drv.c. Suggested-by: Maxime Ripard Signed-off-by: Douglas Anderson --- This commit is only compile-time tested. ...and further, I'd say that this patch is more of a plea for help than a patch I think is actually right. I'm _fairly_ certain that drm/amdgpu needs this call at shutdown time but the logic is a bit hard for me to follow. I'd appreciate if anyone who actually knows what this should look like could illuminate me, or perhaps even just post a patch themselves! drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 2 ++ 3 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 8f2255b3a38a..cfcff0b37466 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1104,6 +1104,7 @@ static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_device *bdev) int amdgpu_device_init(struct amdgpu_device *adev, uint32_t flags); void amdgpu_device_fini_hw(struct amdgpu_device *adev); +void amdgpu_device_shutdown_hw(struct amdgpu_device *adev); void amdgpu_device_fini_sw(struct amdgpu_device *adev); int amdgpu_gpu_wait_for_idle(struct amdgpu_device *adev); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index a2cdde0ca0a7..fa5925c2092d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -4247,6 +4247,16 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev) } +void amdgpu_device_shutdown_hw(struct amdgpu_device *adev) +{ + if (adev->mode_info.mode_config_initialized) { + if (!drm_drv_uses_atomic_modeset(adev_to_drm(adev))) + drm_helper_force_disable_all(adev_to_drm(adev)); + else + drm_atomic_helper_shutdown(adev_to_drm(adev)); + } +} + void amdgpu_device_fini_sw(struct amdgpu_device *adev) { int idx; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index e90f730eb715..3a7cbff111d1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2333,6 +2333,8 @@ amdgpu_pci_shutdown(struct pci_dev *pdev) struct drm_device *dev = pci_get_drvdata(pdev); struct amdgpu_device *adev = drm_to_adev(dev); + amdgpu_device_shutdown_hw(adev); + if (amdgpu_ras_intr_triggered()) return; -- 2.42.0.283.g2d96d420d3-goog
[RFT PATCH 14/15] drm/radeon: Call drm_helper_force_disable_all() at shutdown/remove time
Based on grepping through the source code, this driver appears to be missing a call to drm_atomic_helper_shutdown(), or in this case the non-atomic equivalent drm_helper_force_disable_all(), at system shutdown time and at driver remove time. This is important because drm_helper_force_disable_all() will cause panels to get disabled cleanly which may be important for their power sequencing. Future changes will remove any custom powering off in individual panel drivers so the DRM drivers need to start getting this right. The fact that we should call drm_atomic_helper_shutdown(), or in this case the non-atomic equivalent drm_helper_force_disable_all(), in the case of OS shutdown/restart comes straight out of the kernel doc "driver instance overview" in drm_drv.c. NOTE: in order to get things inserted in the right place, I had to replace the old/deprecated drm_put_dev() function with the equivalent new calls. Suggested-by: Maxime Ripard Signed-off-by: Douglas Anderson --- I honestly have no idea if I got this patch right. The shutdown() function already had some special case logic for PPC, Loongson, and VMs and I don't 100% for sure know how this interacts with those. Everything here is just compile tested. drivers/gpu/drm/radeon/radeon_drv.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 39cdede460b5..67995ea24852 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -38,6 +38,7 @@ #include #include +#include #include #include #include @@ -357,7 +358,9 @@ radeon_pci_remove(struct pci_dev *pdev) { struct drm_device *dev = pci_get_drvdata(pdev); - drm_put_dev(dev); + drm_dev_unregister(dev); + drm_helper_force_disable_all(dev); + drm_dev_put(dev); } static void @@ -368,6 +371,8 @@ radeon_pci_shutdown(struct pci_dev *pdev) */ if (radeon_device_is_virtual()) radeon_pci_remove(pdev); + else + drm_helper_force_disable_all(pci_get_drvdata(pdev)); #if defined(CONFIG_PPC64) || defined(CONFIG_MACH_LOONGSON64) /* -- 2.42.0.283.g2d96d420d3-goog