Re: [PATCH] drm/vkms: fix 32bit compilation error by replacing macros
Hi Mellisa,
Thanks for the patch fixing my mistakes.
On 9/9/22 08:41, Melissa Wen wrote:
Replace vkms_formats macros for fixed-point operations with functions
from drm/drm_fixed.h to do the same job and fix 32-bit compilation
errors.
Fixes: a19c2ac9858 ("drm: vkms: Add support to the RGB565 format")
Tested-by: Sudip Mukherjee
Reported-by: Sudip Mukherjee
Reported-by: kernel test robot
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/vkms/vkms_formats.c | 53 +++--
1 file changed, 19 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c
b/drivers/gpu/drm/vkms/vkms_formats.c
index 300abb4d1dfe..ddcd3cfeeaac 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -1,27 +1,12 @@
// SPDX-License-Identifier: GPL-2.0+
-#include
+#include
#include
+#include
+#include
#include "vkms_formats.h"
-/* The following macros help doing fixed point arithmetic. */
-/*
- * With Fixed-Point scale 15 we have 17 and 15 bits of integer and fractional
- * parts respectively.
- * | 0.000 |
- * 31 0
- */
-#define SHIFT 15
-
-#define INT_TO_FIXED(a) ((a) << SHIFT)
-#define FIXED_MUL(a, b) ((s32)(((s64)(a) * (b)) >> SHIFT))
-#define FIXED_DIV(a, b) ((s32)(((s64)(a) << SHIFT) / (b)))
-/* This macro converts a fixed point number to int, and round half up it */
-#define FIXED_TO_INT_ROUND(a) (((a) + (1 << (SHIFT - 1))) >> SHIFT)
-#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
-#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
-
static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x,
int y)
{
return frame_info->offset + (y * frame_info->pitch)
@@ -137,19 +122,19 @@ static void RGB565_to_argb_u16(struct line_buffer
*stage_buffer,
int x_limit = min_t(size_t, drm_rect_width(_info->dst),
stage_buffer->n_pixels);
- s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
- s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
+ s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
+ s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
I think you need to add `drm_int2fixp` to 31 and 63.
for (size_t x = 0; x < x_limit; x++, src_pixels++) {
u16 rgb_565 = le16_to_cpu(*src_pixels);
- s32 fp_r = INT_TO_FIXED((rgb_565 >> 11) & 0x1f);
- s32 fp_g = INT_TO_FIXED((rgb_565 >> 5) & 0x3f);
- s32 fp_b = INT_TO_FIXED(rgb_565 & 0x1f);
+ s32 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
+ s32 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
+ s32 fp_b = drm_int2fixp(rgb_565 & 0x1f);
And we are cast implicitly from 64 bits int to 32 bits which is
implementation-defined AFAIK. So, probably we should be using `s64` for
all of these variables.
I tested the patch. And I'm seeing some differences in the intermediate
results. From my testing, these changes solve those differences.
Another thing that may have an impact on the final output is the lack of
rounding in drm_fixed.h. This can potentially produce the wrong result.
Thanks,
---
Igor Torrente
out_pixels[x].a = (u16)0x;
- out_pixels[x].r = FIXED_TO_INT_ROUND(FIXED_MUL(fp_r,
fp_rb_ratio));
- out_pixels[x].g = FIXED_TO_INT_ROUND(FIXED_MUL(fp_g,
fp_g_ratio));
- out_pixels[x].b = FIXED_TO_INT_ROUND(FIXED_MUL(fp_b,
fp_rb_ratio));
+ out_pixels[x].r = drm_fixp2int(drm_fixp_mul(fp_r, fp_rb_ratio));
+ out_pixels[x].g = drm_fixp2int(drm_fixp_mul(fp_g, fp_g_ratio));
+ out_pixels[x].b = drm_fixp2int(drm_fixp_mul(fp_b, fp_rb_ratio));
}
}
@@ -248,17 +233,17 @@ static void argb_u16_to_RGB565(struct vkms_frame_info *frame_info,
int x_limit = min_t(size_t, drm_rect_width(_info->dst),
src_buffer->n_pixels);
- s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
- s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
+ s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
+ s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
for (size_t x = 0; x < x_limit; x++, dst_pixels++) {
- s32 fp_r = INT_TO_FIXED(in_pixels[x].r);
- s32 fp_g = INT_TO_FIXED(in_pixels[x].g);
- s32 fp_b = INT_TO_FIXED(in_pixels[x].b);
+ s32 fp_r = drm_int2fixp(in_pixels[x].r);
+ s32 fp_g = drm_int2fixp(in_pixels[x].g);
+ s32 fp_b = drm_int2fixp(in_pixels[x].b);
- u16 r = FIXED_TO_INT_ROUND(FIXED_DIV(fp_r, fp_rb_ratio));
- u16 g = FIXED_TO_INT_ROUND(FIXED_DIV(fp_g, fp_g_ratio));
- u16 b = FIXED_TO_INT_ROUND(FIXED_DIV(fp_b, fp_rb_ratio));
+ u16 r = drm_fixp2int(drm_fixp_div(fp_r, fp_rb_ratio));
+ u16 g =
Re: [PATCH v4 14/15] drm/i915/huc: define gsc-compatible HuC fw for DG2
Nit: wish some of those macro param names were more descriptive, example :
fw_def -> fw_namer or prefix -> gen
But that's irrelevant here, so
Reviewed-by: Alan Previn
On Thu, 2022-09-08 at 17:16 -0700, Ceraolo Spurio, Daniele wrote:
> The fw name is different and we need to record the fact that the blob is
> gsc-loaded, so add a new macro to help.
>
> Note: A-step DG2 G10 does not support HuC loading via GSC and would
> require a separate firmware to be loaded the legacy way, but that's
> not a production stepping so we're not going to bother.
>
> v2: rebase on new fw fetch logic
>
> Signed-off-by: Daniele Ceraolo Spurio
> Cc: Tony Ye
> Reviewed-by: Alan Previn #v1
> ---
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 23 ---
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index 4792960d9c04..09e06ac8bcf1 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -91,7 +91,8 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
> fw_def(BROXTON, 0, guc_mmp(bxt, 70, 1, 1)) \
> fw_def(SKYLAKE, 0, guc_mmp(skl, 70, 1, 1))
>
> -#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp) \
> +#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp, huc_gsc) \
> + fw_def(DG2, 0, huc_gsc(dg2)) \
> fw_def(ALDERLAKE_P, 0, huc_mmp(tgl, 7, 9, 3)) \
> fw_def(ALDERLAKE_S, 0, huc_mmp(tgl, 7, 9, 3)) \
> fw_def(DG1, 0, huc_mmp(dg1, 7, 9, 3)) \
> @@ -137,6 +138,9 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
> #define MAKE_HUC_FW_PATH_BLANK(prefix_) \
> __MAKE_UC_FW_PATH_BLANK(prefix_, "_huc")
>
> +#define MAKE_HUC_FW_PATH_GSC(prefix_) \
> + __MAKE_UC_FW_PATH_BLANK(prefix_, "_huc_gsc")
> +
> #define MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \
> __MAKE_UC_FW_PATH_MMP(prefix_, "_huc_", major_, minor_, patch_)
>
> @@ -149,7 +153,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
> MODULE_FIRMWARE(uc_);
>
> INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH_MAJOR,
> MAKE_GUC_FW_PATH_MMP)
> -INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK,
> MAKE_HUC_FW_PATH_MMP)
> +INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK,
> MAKE_HUC_FW_PATH_MMP, MAKE_HUC_FW_PATH_GSC)
>
> /*
> * The next expansion of the table macros (in __uc_fw_auto_select below)
> provides
> @@ -164,6 +168,7 @@ struct __packed uc_fw_blob {
> u8 major;
> u8 minor;
> u8 patch;
> + bool loaded_via_gsc;
> };
>
> #define UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
> @@ -172,16 +177,16 @@ struct __packed uc_fw_blob {
> .patch = patch_, \
> .path = path_,
>
> -#define UC_FW_BLOB_NEW(major_, minor_, patch_, path_) \
> +#define UC_FW_BLOB_NEW(major_, minor_, patch_, gsc_, path_) \
> { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
> - .legacy = false }
> + .legacy = false, .loaded_via_gsc = gsc_ }
>
> #define UC_FW_BLOB_OLD(major_, minor_, patch_, path_) \
> { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
> .legacy = true }
>
> #define GUC_FW_BLOB(prefix_, major_, minor_) \
> - UC_FW_BLOB_NEW(major_, minor_, 0, \
> + UC_FW_BLOB_NEW(major_, minor_, 0, false, \
> MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_))
>
> #define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \
> @@ -189,12 +194,15 @@ struct __packed uc_fw_blob {
> MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_))
>
> #define HUC_FW_BLOB(prefix_) \
> - UC_FW_BLOB_NEW(0, 0, 0, MAKE_HUC_FW_PATH_BLANK(prefix_))
> + UC_FW_BLOB_NEW(0, 0, 0, false, MAKE_HUC_FW_PATH_BLANK(prefix_))
>
> #define HUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \
> UC_FW_BLOB_OLD(major_, minor_, patch_, \
> MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_))
>
> +#define HUC_FW_BLOB_GSC(prefix_) \
> + UC_FW_BLOB_NEW(0, 0, 0, true, MAKE_HUC_FW_PATH_GSC(prefix_))
> +
> struct __packed uc_fw_platform_requirement {
> enum intel_platform p;
> u8 rev; /* first platform rev using this FW */
> @@ -220,7 +228,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct
> intel_uc_fw *uc_fw)
> INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
> GUC_FW_BLOB_MMP)
> };
> static const struct uc_fw_platform_requirement blobs_huc[] = {
> - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
> HUC_FW_BLOB_MMP)
> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
> };
> static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] =
> {
> [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
> @@ -266,6 +274,7 @@
[PATCH] drm/i915/mtl: Add MTL forcewake support
MTL has separate forcewake tables for the primary/render GT and the
media GT; each GT's intel_uncore will use a separate forcewake table and
should only initialize the domains that are relevant to that GT. The GT
ack register also moves to a new location of (GSI base + 0xDFC) on this
platform.
Note that although our uncore handlers take care of transparently
redirecting all register accesses in the media GT's GSI range to their
new offset at 0x38, the forcewake ranges listed in the table should
use the final, post-translation offsets.
NOTE: There are two ranges in the media IP that have multicast
registers where the two register instances reside in different power
wells (either VD0 or VD2). We don't have an easy way to deal with this
today (and in fact we don't even access these register ranges in the
driver today), so for now we just mark those ranges as FORCEWAKE_ALL
which will cause all of the media power wells to be grabbed, ensuring
proper operation. If we start reading/writing in those ranges in the
future, we can re-visit whether it's worth adding extra steering
complexity into our forcewake support.
Bspec: 67788, 67789, 52077
Cc: Radhakrishna Sripada
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/intel_gt_regs.h | 5 +
drivers/gpu/drm/i915/intel_uncore.c | 258 +-
drivers/gpu/drm/i915/intel_uncore.h | 2 +
drivers/gpu/drm/i915/selftests/intel_uncore.c | 4 +
4 files changed, 258 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 2275ee47da95..1cbb7226400b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -39,6 +39,9 @@
#define FORCEWAKE_ACK_RENDER_GEN9 _MMIO(0xd84)
#define FORCEWAKE_ACK_MEDIA_GEN9 _MMIO(0xd88)
+#define FORCEWAKE_ACK_GSC _MMIO(0xdf8)
+#define FORCEWAKE_ACK_GT_MTL _MMIO(0xdfc)
+
#define MCFG_MCR_SELECTOR _MMIO(0xfd0)
#define SF_MCR_SELECTOR_MMIO(0xfd8)
#define GEN8_MCR_SELECTOR _MMIO(0xfdc)
@@ -898,6 +901,8 @@
#define FORCEWAKE_MEDIA_VDBOX_GEN11(n) _MMIO(0xa540 + (n) * 4)
#define FORCEWAKE_MEDIA_VEBOX_GEN11(n) _MMIO(0xa560 + (n) * 4)
+#define FORCEWAKE_REQ_GSC _MMIO(0xa618)
+
#define CHV_POWER_SS0_SIG1 _MMIO(0xa720)
#define CHV_POWER_SS0_SIG2 _MMIO(0xa724)
#define CHV_POWER_SS1_SIG1 _MMIO(0xa728)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 5cd423c7b646..c058cdc6d8a0 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -104,6 +104,7 @@ static const char * const forcewake_domain_names[] = {
"vebox1",
"vebox2",
"vebox3",
+ "gsc",
};
const char *
@@ -888,10 +889,13 @@ void assert_forcewakes_active(struct intel_uncore *uncore,
spin_unlock_irq(>lock);
}
-/* We give fast paths for the really cool registers */
+/*
+ * We give fast paths for the really cool registers. The second range includes
+ * media domains (and the GSC starting from Xe_LPM+)
+ */
#define NEEDS_FORCE_WAKE(reg) ({ \
u32 __reg = (reg); \
- __reg < 0x4 || __reg >= GEN11_BSD_RING_BASE; \
+ __reg < 0x4 || __reg >= 0x116000; \
})
static int fw_range_cmp(u32 offset, const struct intel_forcewake_range *entry)
@@ -1131,6 +1135,45 @@ static const struct i915_range pvc_shadowed_regs[] = {
{ .start = 0x1F8510, .end = 0x1F8550 },
};
+static const struct i915_range mtl_shadowed_regs[] = {
+ { .start = 0x2030, .end = 0x2030 },
+ { .start = 0x2510, .end = 0x2550 },
+ { .start = 0xA008, .end = 0xA00C },
+ { .start = 0xA188, .end = 0xA188 },
+ { .start = 0xA278, .end = 0xA278 },
+ { .start = 0xA540, .end = 0xA56C },
+ { .start = 0xC050, .end = 0xC050 },
+ { .start = 0xC340, .end = 0xC340 },
+ { .start = 0xC4C8, .end = 0xC4C8 },
+ { .start = 0xC4E0, .end = 0xC4E0 },
+ { .start = 0xC600, .end = 0xC600 },
+ { .start = 0xC658, .end = 0xC658 },
+ { .start = 0xCFD4, .end = 0xCFDC },
+ { .start = 0x22030, .end = 0x22030 },
+ { .start = 0x22510, .end = 0x22550 },
+};
+
+static const struct i915_range xelpmp_shadowed_regs[] = {
+ { .start = 0x1C0030, .end = 0x1C0030 },
+ { .start = 0x1C0510, .end = 0x1C0550 },
+ { .start = 0x1C8030, .end = 0x1C8030 },
+ { .start = 0x1C8510, .end = 0x1C8550 },
+ { .start = 0x1D0030, .end = 0x1D0030 },
+ { .start = 0x1D0510, .end = 0x1D0550 },
+ { .start = 0x38A008, .end = 0x38A00C },
+ { .start = 0x38A188, .end = 0x38A188 },
+ { .start = 0x38A278, .end = 0x38A278 },
+ { .start = 0x38A540,
[PATCH 3/3] drm/i915/rps: Freq caps for MTL
For MTL, when reading from HW, RP0, RP1 (actuall RPe) and RPn freq use an
entirely different set of registers with different fields, bitwidths and
units.
v2: Move MTL check into a separate function (Jani)
Cc: Jani Nikula
Cc: Badal Nilawar
Signed-off-by: Ashutosh Dixit
Reviewed-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/gt/intel_rps.c | 46 +++--
drivers/gpu/drm/i915/i915_reg.h | 9 ++
2 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 6b86250c31ab..17b40b625e31 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1085,15 +1085,25 @@ static u32 intel_rps_read_state_cap(struct intel_rps
*rps)
return intel_uncore_read(uncore, GEN6_RP_STATE_CAP);
}
-/**
- * gen6_rps_get_freq_caps - Get freq caps exposed by HW
- * @rps: the intel_rps structure
- * @caps: returned freq caps
- *
- * Returned "caps" frequencies should be converted to MHz using
- * intel_gpu_freq()
- */
-void gen6_rps_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps
*caps)
+static void
+mtl_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps *caps)
+{
+ struct intel_uncore *uncore = rps_to_uncore(rps);
+ u32 rp_state_cap = rps_to_gt(rps)->type == GT_MEDIA ?
+ intel_uncore_read(uncore, MTL_MEDIAP_STATE_CAP)
:
+ intel_uncore_read(uncore, MTL_RP_STATE_CAP);
+ u32 rpe = rps_to_gt(rps)->type == GT_MEDIA ?
+ intel_uncore_read(uncore, MTL_MPE_FREQUENCY) :
+ intel_uncore_read(uncore, MTL_GT_RPE_FREQUENCY);
+
+ /* MTL values are in units of 16.67 MHz */
+ caps->rp0_freq = REG_FIELD_GET(MTL_RP0_CAP_MASK, rp_state_cap);
+ caps->min_freq = REG_FIELD_GET(MTL_RPN_CAP_MASK, rp_state_cap);
+ caps->rp1_freq = REG_FIELD_GET(MTL_RPE_MASK, rpe);
+}
+
+static void
+__gen6_rps_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps
*caps)
{
struct drm_i915_private *i915 = rps_to_i915(rps);
u32 rp_state_cap;
@@ -1128,6 +1138,24 @@ void gen6_rps_get_freq_caps(struct intel_rps *rps,
struct intel_rps_freq_caps *c
}
}
+/**
+ * gen6_rps_get_freq_caps - Get freq caps exposed by HW
+ * @rps: the intel_rps structure
+ * @caps: returned freq caps
+ *
+ * Returned "caps" frequencies should be converted to MHz using
+ * intel_gpu_freq()
+ */
+void gen6_rps_get_freq_caps(struct intel_rps *rps, struct intel_rps_freq_caps
*caps)
+{
+ struct drm_i915_private *i915 = rps_to_i915(rps);
+
+ if (IS_METEORLAKE(i915))
+ return mtl_get_freq_caps(rps, caps);
+ else
+ return __gen6_rps_get_freq_caps(rps, caps);
+}
+
static void gen6_rps_init(struct intel_rps *rps)
{
struct drm_i915_private *i915 = rps_to_i915(rps);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 38e895ad4b59..2101b6d6dae5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1792,6 +1792,15 @@
#define XEHPSDV_RP_STATE_CAP _MMIO(0x250014)
#define PVC_RP_STATE_CAP _MMIO(0x281014)
+#define MTL_RP_STATE_CAP _MMIO(0x138000)
+#define MTL_MEDIAP_STATE_CAP _MMIO(0x138020)
+#define MTL_RP0_CAP_MASK REG_GENMASK(8, 0)
+#define MTL_RPN_CAP_MASK REG_GENMASK(24, 16)
+
+#define MTL_GT_RPE_FREQUENCY _MMIO(0x13800c)
+#define MTL_MPE_FREQUENCY _MMIO(0x13802c)
+#define MTL_RPE_MASK REG_GENMASK(8, 0)
+
#define GT0_PERF_LIMIT_REASONS _MMIO(0x1381a8)
#define GT0_PERF_LIMIT_REASONS_MASK 0xde3
#define PROCHOT_MASK REG_BIT(0)
--
2.34.1
[PATCH 1/3] drm/i915/debugfs: Add perf_limit_reasons in debugfs
From: Tilak Tangudu
Add perf_limit_reasons in debugfs. The upper 16 perf_limit_reasons RW "log"
bits are identical to the lower 16 RO "status" bits except that the "log"
bits remain set until cleared, thereby ensuring the throttling occurrence
is not missed. The clear fop clears the upper 16 "log" bits, the get fop
gets all 32 "log" and "status" bits.
v2: Expand commit message and clarify "log" and "status" bits in
comment (Rodrigo)
Cc: Rodrigo Vivi
Signed-off-by: Ashutosh Dixit
Signed-off-by: Tilak Tangudu
Reviewed-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 31 +++
drivers/gpu/drm/i915/i915_reg.h | 1 +
2 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
index 108b9e76c32e..a009cf69103a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
@@ -655,6 +655,36 @@ static bool rps_eval(void *data)
DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(rps_boost);
+static int perf_limit_reasons_get(void *data, u64 *val)
+{
+ struct intel_gt *gt = data;
+ intel_wakeref_t wakeref;
+
+ with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+ *val = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
+
+ return 0;
+}
+
+static int perf_limit_reasons_clear(void *data, u64 val)
+{
+ struct intel_gt *gt = data;
+ intel_wakeref_t wakeref;
+
+ /*
+* Clear the upper 16 "log" bits, the lower 16 "status" bits are
+* read-only. The upper 16 "log" bits are identical to the lower 16
+* "status" bits except that the "log" bits remain set until cleared.
+*/
+ with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+ intel_uncore_rmw(gt->uncore, GT0_PERF_LIMIT_REASONS,
+GT0_PERF_LIMIT_REASONS_LOG_MASK, 0);
+
+ return 0;
+}
+DEFINE_SIMPLE_ATTRIBUTE(perf_limit_reasons_fops, perf_limit_reasons_get,
+ perf_limit_reasons_clear, "%llu\n");
+
void intel_gt_pm_debugfs_register(struct intel_gt *gt, struct dentry *root)
{
static const struct intel_gt_debugfs_file files[] = {
@@ -664,6 +694,7 @@ void intel_gt_pm_debugfs_register(struct intel_gt *gt,
struct dentry *root)
{ "forcewake_user", _user_fops, NULL},
{ "llc", _fops, llc_eval },
{ "rps_boost", _boost_fops, rps_eval },
+ { "perf_limit_reasons", _limit_reasons_fops, NULL },
};
intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), gt);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 52462cbfdc66..58b0ed95 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1802,6 +1802,7 @@
#define POWER_LIMIT_4_MASK REG_BIT(8)
#define POWER_LIMIT_1_MASK REG_BIT(10)
#define POWER_LIMIT_2_MASK REG_BIT(11)
+#define GT0_PERF_LIMIT_REASONS_LOG_MASK REG_GENMASK(31, 16)
#define CHV_CLK_CTL1 _MMIO(0x101100)
#define VLV_CLK_CTL2 _MMIO(0x101104)
--
2.34.1
[PATCH 2/3] drm/i915/mtl: PERF_LIMIT_REASONS changes for MTL
PERF_LIMIT_REASONS register for MTL media gt is different now.
v2: Avoid static inline for intel_gt_perf_limit_reasons_reg() (Jani)
Cc: Jani Nikula
Cc: Badal Nilawar
Signed-off-by: Ashutosh Dixit
Reviewed-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/gt/intel_gt.c| 6 ++
drivers/gpu/drm/i915/gt/intel_gt.h| 1 +
drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 6 +++---
drivers/gpu/drm/i915/i915_reg.h | 1 +
5 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index b59fb03ed274..46929afa18c2 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -229,6 +229,12 @@ static void gen6_clear_engine_error_register(struct
intel_engine_cs *engine)
GEN6_RING_FAULT_REG_POSTING_READ(engine);
}
+i915_reg_t intel_gt_perf_limit_reasons_reg(struct intel_gt *gt)
+{
+ return gt->type == GT_MEDIA ?
+ MTL_MEDIA_PERF_LIMIT_REASONS : GT0_PERF_LIMIT_REASONS;
+}
+
void
intel_gt_clear_error_registers(struct intel_gt *gt,
intel_engine_mask_t engine_mask)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h
b/drivers/gpu/drm/i915/gt/intel_gt.h
index 2ee582e287c8..e0365d556248 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -60,6 +60,7 @@ void intel_gt_driver_late_release_all(struct drm_i915_private
*i915);
int intel_gt_wait_for_idle(struct intel_gt *gt, long timeout);
void intel_gt_check_and_clear_faults(struct intel_gt *gt);
+i915_reg_t intel_gt_perf_limit_reasons_reg(struct intel_gt *gt);
void intel_gt_clear_error_registers(struct intel_gt *gt,
intel_engine_mask_t engine_mask);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
index a009cf69103a..68310881a793 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
@@ -661,7 +661,7 @@ static int perf_limit_reasons_get(void *data, u64 *val)
intel_wakeref_t wakeref;
with_intel_runtime_pm(gt->uncore->rpm, wakeref)
- *val = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
+ *val = intel_uncore_read(gt->uncore,
intel_gt_perf_limit_reasons_reg(gt));
return 0;
}
@@ -677,7 +677,7 @@ static int perf_limit_reasons_clear(void *data, u64 val)
* "status" bits except that the "log" bits remain set until cleared.
*/
with_intel_runtime_pm(gt->uncore->rpm, wakeref)
- intel_uncore_rmw(gt->uncore, GT0_PERF_LIMIT_REASONS,
+ intel_uncore_rmw(gt->uncore,
intel_gt_perf_limit_reasons_reg(gt),
GT0_PERF_LIMIT_REASONS_LOG_MASK, 0);
return 0;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
index e066cc33d9f2..54deae45d81f 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -510,7 +510,7 @@ struct intel_gt_bool_throttle_attr {
struct attribute attr;
ssize_t (*show)(struct device *dev, struct device_attribute *attr,
char *buf);
- i915_reg_t reg32;
+ i915_reg_t (*reg32)(struct intel_gt *gt);
u32 mask;
};
@@ -521,7 +521,7 @@ static ssize_t throttle_reason_bool_show(struct device *dev,
struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
struct intel_gt_bool_throttle_attr *t_attr =
(struct intel_gt_bool_throttle_attr *) attr;
- bool val = rps_read_mask_mmio(>rps, t_attr->reg32, t_attr->mask);
+ bool val = rps_read_mask_mmio(>rps, t_attr->reg32(gt),
t_attr->mask);
return sysfs_emit(buff, "%u\n", val);
}
@@ -530,7 +530,7 @@ static ssize_t throttle_reason_bool_show(struct device *dev,
struct intel_gt_bool_throttle_attr attr_##sysfs_func__ = { \
.attr = { .name = __stringify(sysfs_func__), .mode = 0444 }, \
.show = throttle_reason_bool_show, \
- .reg32 = GT0_PERF_LIMIT_REASONS, \
+ .reg32 = intel_gt_perf_limit_reasons_reg, \
.mask = mask__, \
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 58b0ed95..38e895ad4b59 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1803,6 +1803,7 @@
#define POWER_LIMIT_1_MASK REG_BIT(10)
#define POWER_LIMIT_2_MASK REG_BIT(11)
#define GT0_PERF_LIMIT_REASONS_LOG_MASK REG_GENMASK(31, 16)
+#define MTL_MEDIA_PERF_LIMIT_REASONS _MMIO(0x138030)
#define CHV_CLK_CTL1 _MMIO(0x101100)
#define VLV_CLK_CTL2 _MMIO(0x101104)
--
2.34.1
[PATCH 0/3] i915: freq caps and perf_limit_reasons changes for MTL
Since https://patchwork.freedesktop.org/series/107908/ is now merged, rebase this series on latest drm-tip and post a clean series. Ashutosh Dixit (2): drm/i915/mtl: PERF_LIMIT_REASONS changes for MTL drm/i915/rps: Freq caps for MTL Tilak Tangudu (1): drm/i915/debugfs: Add perf_limit_reasons in debugfs drivers/gpu/drm/i915/gt/intel_gt.c| 6 +++ drivers/gpu/drm/i915/gt/intel_gt.h| 1 + drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 31 + drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 6 +-- drivers/gpu/drm/i915/gt/intel_rps.c | 46 +++ drivers/gpu/drm/i915/i915_reg.h | 11 + 6 files changed, 89 insertions(+), 12 deletions(-) -- 2.34.1
Re: [PATCH v3 01/15] vfio: Add helpers for unifying vfio_device life cycle
On Fri, Sep 09, 2022 at 08:42:25AM +, Tian, Kevin wrote: > I think it's quite common to have an alloc() helper initialize refcount, e.g. > vfio_group_alloc() both initialize its user refcount and also call > device_initialize() to gets kref initialized. Similar example in > ib_alloc_device(), etc. Right, it is quite a good/common pattern to have an allocation function return a refcount to the caller. I don't know of any naming standard for this however. Jason
Re: [PATCH v6 28/57] drm_print: refine drm_debug_enabled for jump-label
On Wed, Sep 7, 2022 at 12:40 AM Daniel Vetter wrote: > > On Sun, Sep 04, 2022 at 03:41:05PM -0600, Jim Cromie wrote: > > In order to use dynamic-debug's jump-label optimization in drm-debug, > > its clarifying to refine drm_debug_enabled into 3 uses: > > > > 1. drm_debug_enabled - legacy, public > > 2. __drm_debug_enabled - optimized for dyndbg jump-label enablement. > > 3. _drm_debug_enabled - pr_debug instrumented, observable > > > > 1. The legacy version always checks the bits. > > > > 2. is privileged, for use by __drm_dbg(), __drm_dev_dbg(), which do an > > early return unless the category is enabled. For dyndbg builds, debug > > callsites are selectively "pre-enabled", so __drm_debug_enabled() > > short-circuits to true there. Remaining callers of 1 may be able to > > use 2, case by case. > > > > 3. is 1st wrapped in a macro, with a pr_debug, which reports each > > usage in /proc/dynamic_debug/control, making it observable in the > > logs. The macro lets the pr_debug see the real caller, not an inline > > function. > > > > When plugged into 1, 3 identified ~10 remaining callers of the > > function, leading to the follow-on cleanup patch, and would allow > > activating the pr_debugs, estimating the callrate, and the potential > > savings by using the wrapper macro. It is unused ATM, but it fills > > out the picture. > > > > Signed-off-by: Jim Cromie > > So instead of having 3 here as a "you need to hack it in to see what > should be converted" I have a bit a different idea: Could we make the > public version also a dyndbg callsite (like the printing wrappers), but > instead of a dynamic call we'd have a dynamically fixed value we get out? > I think that would take care of everything you have here as an open. > > Otherwise I'd just drop 3 for the series we're going to merge. > -Daniel > OK - So here it is in use again, with modules drm amdgpu i915 loaded + deps :#> grep todo /proc/dynamic_debug/control drivers/gpu/drm/drm_edid_load.c:178 [drm]edid_load =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/drm_vblank.c:410 [drm]drm_crtc_accurate_vblank_count =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/drm_vblank.c:787 [drm]drm_crtc_vblank_helper_get_vblank_timestamp_internal =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/drm_vblank.c:1491 [drm]drm_vblank_restore =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/drm_vblank.c:1433 [drm]drm_vblank_enable =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/drm_plane.c:2168 [drm]drm_mode_setplane =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/display/drm_dp_mst_topology.c:1359 [drm_display_helper]drm_dp_mst_wait_tx_reply =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/display/drm_dp_mst_topology.c:2864 [drm_display_helper]process_single_tx_qlock =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/display/drm_dp_mst_topology.c:2909 [drm_display_helper]drm_dp_queue_down_tx =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/display/drm_dp_mst_topology.c:1686 [drm_display_helper]drm_dp_mst_update_slots =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/i915/display/intel_dp.c: [i915]intel_dp_print_rates =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/i915/display/intel_backlight.c:5434 [i915]cnp_enable_backlight =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/i915/display/intel_backlight.c:5459 [i915]intel_backlight_device_register =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/i915/display/intel_opregion.c:43 [i915]intel_opregion_notify_encoder =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/i915/display/intel_opregion.c:53 [i915]asle_set_backlight =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/i915/display/intel_bios.c:1088 [i915]intel_bios_is_dsi_present =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/i915/display/intel_display_debugfs.c:6153 [i915]i915_drrs_ctl_set =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/i915/intel_pcode.c:26 [i915]snb_pcode_read =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/i915/i915_getparam.c:785 [i915]i915_getparam_ioctl =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c:282 [amdgpu]vcn_v2_5_process_interrupt =_ "todo: maybe avoid via dyndbg\n" drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c:433 [amdgpu]vcn_v2_0_process_interrupt =_ "todo: maybe avoid via dyndbg\n" w/o actually looking, the vblank debug could be called frequently. I'll build on my amdgpu box to run on real hardware. And Im inclined to restore the instrumented version (with the "todo:") care to suggest a better message than "maybe avoid" ?
[PATCH v2 2/2] drm/i915/gt: Extract function to apply media fuses
Just like is done for compute and copy engines, extract a function to
handle media engines. While at it, be consistent on using or not the
uncore/gt/info variable aliases.
Reviewed-by: Matt Roper
Signed-off-by: Lucas De Marchi
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index b6602439224d..814f83b5fe59 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -663,6 +663,74 @@ bool gen11_vdbox_has_sfc(struct intel_gt *gt,
return false;
}
+static void engine_mask_apply_media_fuses(struct intel_gt *gt)
+{
+ struct drm_i915_private *i915 = gt->i915;
+ unsigned int logical_vdbox = 0;
+ unsigned int i;
+ u32 media_fuse, fuse1;
+ u16 vdbox_mask;
+ u16 vebox_mask;
+
+ if (MEDIA_VER(gt->i915) < 11)
+ return;
+
+ /*
+* On newer platforms the fusing register is called 'enable' and has
+* enable semantics, while on older platforms it is called 'disable'
+* and bits have disable semantices.
+*/
+ media_fuse = intel_uncore_read(gt->uncore,
GEN11_GT_VEBOX_VDBOX_DISABLE);
+ if (MEDIA_VER_FULL(i915) < IP_VER(12, 50))
+ media_fuse = ~media_fuse;
+
+ vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK;
+ vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >>
+ GEN11_GT_VEBOX_DISABLE_SHIFT;
+
+ if (MEDIA_VER_FULL(i915) >= IP_VER(12, 50)) {
+ fuse1 = intel_uncore_read(gt->uncore, HSW_PAVP_FUSE1);
+ gt->info.sfc_mask = REG_FIELD_GET(XEHP_SFC_ENABLE_MASK, fuse1);
+ } else {
+ gt->info.sfc_mask = ~0;
+ }
+
+ for (i = 0; i < I915_MAX_VCS; i++) {
+ if (!HAS_ENGINE(gt, _VCS(i))) {
+ vdbox_mask &= ~BIT(i);
+ continue;
+ }
+
+ if (!(BIT(i) & vdbox_mask)) {
+ gt->info.engine_mask &= ~BIT(_VCS(i));
+ drm_dbg(>drm, "vcs%u fused off\n", i);
+ continue;
+ }
+
+ if (gen11_vdbox_has_sfc(gt, i, logical_vdbox, vdbox_mask))
+ gt->info.vdbox_sfc_access |= BIT(i);
+ logical_vdbox++;
+ }
+ drm_dbg(>drm, "vdbox enable: %04x, instances: %04lx\n",
+ vdbox_mask, VDBOX_MASK(gt));
+ GEM_BUG_ON(vdbox_mask != VDBOX_MASK(gt));
+
+ for (i = 0; i < I915_MAX_VECS; i++) {
+ if (!HAS_ENGINE(gt, _VECS(i))) {
+ vebox_mask &= ~BIT(i);
+ continue;
+ }
+
+ if (!(BIT(i) & vebox_mask)) {
+ gt->info.engine_mask &= ~BIT(_VECS(i));
+ drm_dbg(>drm, "vecs%u fused off\n", i);
+ }
+ }
+ drm_dbg(>drm, "vebox enable: %04x, instances: %04lx\n",
+ vebox_mask, VEBOX_MASK(gt));
+ GEM_BUG_ON(vebox_mask != VEBOX_MASK(gt));
+}
+
static void engine_mask_apply_compute_fuses(struct intel_gt *gt)
{
struct drm_i915_private *i915 = gt->i915;
@@ -671,6 +739,9 @@ static void engine_mask_apply_compute_fuses(struct intel_gt
*gt)
unsigned long ccs_mask;
unsigned int i;
+ if (GRAPHICS_VER(i915) < 11)
+ return;
+
if (hweight32(CCS_MASK(gt)) <= 1)
return;
@@ -726,75 +797,11 @@ static void engine_mask_apply_copy_fuses(struct intel_gt
*gt)
*/
static intel_engine_mask_t init_engine_mask(struct intel_gt *gt)
{
- struct drm_i915_private *i915 = gt->i915;
struct intel_gt_info *info = >info;
- struct intel_uncore *uncore = gt->uncore;
- unsigned int logical_vdbox = 0;
- unsigned int i;
- u32 media_fuse, fuse1;
- u16 vdbox_mask;
- u16 vebox_mask;
GEM_BUG_ON(!info->engine_mask);
- if (GRAPHICS_VER(i915) < 11)
- return info->engine_mask;
-
- /*
-* On newer platforms the fusing register is called 'enable' and has
-* enable semantics, while on older platforms it is called 'disable'
-* and bits have disable semantices.
-*/
- media_fuse = intel_uncore_read(uncore, GEN11_GT_VEBOX_VDBOX_DISABLE);
- if (MEDIA_VER_FULL(i915) < IP_VER(12, 50))
- media_fuse = ~media_fuse;
-
- vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK;
- vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >>
- GEN11_GT_VEBOX_DISABLE_SHIFT;
-
- if (MEDIA_VER_FULL(i915) >= IP_VER(12, 50)) {
- fuse1 = intel_uncore_read(uncore, HSW_PAVP_FUSE1);
- gt->info.sfc_mask = REG_FIELD_GET(XEHP_SFC_ENABLE_MASK, fuse1);
- } else {
- gt->info.sfc_mask = ~0;
- }
-
- for (i = 0; i < I915_MAX_VCS; i++) {
- if (!HAS_ENGINE(gt, _VCS(i))) {
-
[PATCH v2 0/2] drm/i915: Media fuses future-proofing
Update fuse handling for media to future-proof it. Signed-off-by: Lucas De Marchi --- Lucas De Marchi (2): drm/i915/gt: Use MEDIA_VER() when handling media fuses drm/i915/gt: Extract function to apply media fuses drivers/gpu/drm/i915/gt/intel_engine_cs.c | 142 -- 1 file changed, 74 insertions(+), 68 deletions(-) --- base-commit: ff8b32fbe64a79b380b1cca4232d30c0b29df069 change-id: 20220909-media-87cd0701e0d8 Best regards, -- Lucas De Marchi
[PATCH v2 1/2] drm/i915/gt: Use MEDIA_VER() when handling media fuses
Check for media IP version instead of graphics since this is figuring
out the media engines' configuration. Currently the only platform with
non-matching graphics/media version is Meteor Lake: update the check in
gen11_vdbox_has_sfc() so it considers not only version 12, but also any
later version which then includes that platform.
Reviewed-by: Matt Roper
Signed-off-by: Lucas De Marchi
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 6e0122b3dca2..b6602439224d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -654,13 +654,12 @@ bool gen11_vdbox_has_sfc(struct intel_gt *gt,
*/
if ((gt->info.sfc_mask & BIT(physical_vdbox / 2)) == 0)
return false;
- else if (GRAPHICS_VER(i915) == 12)
+ else if (MEDIA_VER(i915) >= 12)
return (physical_vdbox % 2 == 0) ||
!(BIT(physical_vdbox - 1) & vdbox_mask);
- else if (GRAPHICS_VER(i915) == 11)
+ else if (MEDIA_VER(i915) == 11)
return logical_vdbox % 2 == 0;
- MISSING_CASE(GRAPHICS_VER(i915));
return false;
}
@@ -747,14 +746,14 @@ static intel_engine_mask_t init_engine_mask(struct
intel_gt *gt)
* and bits have disable semantices.
*/
media_fuse = intel_uncore_read(uncore, GEN11_GT_VEBOX_VDBOX_DISABLE);
- if (GRAPHICS_VER_FULL(i915) < IP_VER(12, 50))
+ if (MEDIA_VER_FULL(i915) < IP_VER(12, 50))
media_fuse = ~media_fuse;
vdbox_mask = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK;
vebox_mask = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >>
GEN11_GT_VEBOX_DISABLE_SHIFT;
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
+ if (MEDIA_VER_FULL(i915) >= IP_VER(12, 50)) {
fuse1 = intel_uncore_read(uncore, HSW_PAVP_FUSE1);
gt->info.sfc_mask = REG_FIELD_GET(XEHP_SFC_ENABLE_MASK, fuse1);
} else {
--
b4 0.10.0-dev-df873
Re: [PATCH v6 01/12] dt-bindings: display/msm: split qcom, mdss bindings
On Thu, Sep 8, 2022 at 3:20 PM Dmitry Baryshkov wrote: > > On 08/09/2022 22:37, Rob Herring wrote: > > On Thu, Sep 08, 2022 at 03:37:38PM +0200, Krzysztof Kozlowski wrote: > >> On 01/09/2022 12:23, Dmitry Baryshkov wrote: > >>> Split Mobile Display SubSystem (MDSS) root node bindings to the separate > >>> yaml file. Changes to the existing (txt) schema: > >>> - Added optional "vbif_nrt_phys" region used by msm8996 > >>> - Made "bus" and "vsync" clocks optional (they are not used by some > >>> platforms) > >>> - Added (optional) "core" clock added recently to the mdss driver > >>> - Added optional resets property referencing MDSS reset > >>> - Defined child nodes pointing to corresponding reference schema. > >>> - Dropped the "lut" clock. It was added to the schema by mistake (it is > >>> a part of mdp4 schema, not the mdss). > >>> > >>> Signed-off-by: Dmitry Baryshkov > >>> --- > >>> .../devicetree/bindings/display/msm/mdp5.txt | 30 +--- > >>> .../devicetree/bindings/display/msm/mdss.yaml | 166 ++ > >>> 2 files changed, 167 insertions(+), 29 deletions(-) > >>> create mode 100644 > >>> Documentation/devicetree/bindings/display/msm/mdss.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/display/msm/mdp5.txt > >>> b/Documentation/devicetree/bindings/display/msm/mdp5.txt > >>> index 43d11279c925..65d03c58dee6 100644 > >>> --- a/Documentation/devicetree/bindings/display/msm/mdp5.txt > >>> +++ b/Documentation/devicetree/bindings/display/msm/mdp5.txt > >>> @@ -2,37 +2,9 @@ Qualcomm adreno/snapdragon MDP5 display controller > >>> > >>> Description: > >>> > >>> -This is the bindings documentation for the Mobile Display Subsytem(MDSS) > >>> that > >>> -encapsulates sub-blocks like MDP5, DSI, HDMI, eDP etc, and the MDP5 > >>> display > >>> +This is the bindings documentation for the MDP5 display > >>> controller found in SoCs like MSM8974, APQ8084, MSM8916, MSM8994 and > >>> MSM8996. > >>> > >>> -MDSS: > >>> -Required properties: > >>> -- compatible: > >>> - * "qcom,mdss" - MDSS > >>> -- reg: Physical base address and length of the controller's registers. > >>> -- reg-names: The names of register regions. The following regions are > >>> required: > >>> - * "mdss_phys" > >>> - * "vbif_phys" > >>> -- interrupts: The interrupt signal from MDSS. > >>> -- interrupt-controller: identifies the node as an interrupt controller. > >>> -- #interrupt-cells: specifies the number of cells needed to encode an > >>> interrupt > >>> - source, should be 1. > >>> -- power-domains: a power domain consumer specifier according to > >>> - Documentation/devicetree/bindings/power/power_domain.txt > >>> -- clocks: device clocks. See ../clocks/clock-bindings.txt for details. > >>> -- clock-names: the following clocks are required. > >>> - * "iface" > >>> - * "bus" > >>> - * "vsync" > >>> -- #address-cells: number of address cells for the MDSS children. Should > >>> be 1. > >>> -- #size-cells: Should be 1. > >>> -- ranges: parent bus address space is the same as the child bus address > >>> space. > >>> - > >>> -Optional properties: > >>> -- clock-names: the following clocks are optional: > >>> - * "lut" > >>> - > >>> MDP5: > >>> Required properties: > >>> - compatible: > >>> diff --git a/Documentation/devicetree/bindings/display/msm/mdss.yaml > >>> b/Documentation/devicetree/bindings/display/msm/mdss.yaml > >>> new file mode 100644 > >>> index ..8860fc55cca5 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/display/msm/mdss.yaml > >>> @@ -0,0 +1,166 @@ > >>> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/display/msm/mdss.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: Qualcomm Mobile Display SubSystem (MDSS) > >>> + > >>> +maintainers: > >>> + - Dmitry Baryshkov > >>> + - Rob Clark > >>> + > >>> +description: > >>> + This is the bindings documentation for the Mobile Display > >>> Subsytem(MDSS) that > >>> + encapsulates sub-blocks like MDP5, DSI, HDMI, eDP, etc. > >>> + > >>> +properties: > >>> + compatible: > >>> +enum: > >>> + - qcom,mdss > >>> + > >>> + reg: > >>> +minItems: 2 > >>> +maxItems: 3 > >>> + > >>> + reg-names: > >>> +minItems: 2 > >>> +items: > >>> + - const: mdss_phys > >>> + - const: vbif_phys > >>> + - const: vbif_nrt_phys > >>> + > >>> + interrupts: > >>> +maxItems: 1 > >>> + > >>> + interrupt-controller: > >>> +true > >> > >> If there is going to be v7 - please make it one line. > >> > >>> + > >>> + "#interrupt-cells": > >>> +const: 1 > >>> + > >>> + power-domains: > >>> +maxItems: 1 > >>> +description: | > >>> + The MDSS power domain provided by GCC > >>> + > >>> + clocks: > >>> +minItems: 1 > >>> +items: > >>> + - description: Display abh clock > >>> + - description: Display axi clock > >>>
Re: [PATCH v2 2/2] pci_ids: Add the various Microsoft PCI device IDs
On 9/9/22 19:38, Bjorn Helgaas wrote:
Please follow the PCI subject line conventions. Discover it with
"git log --oneline include/linux/pci_ids.h".
On Fri, Sep 09, 2022 at 11:50:25AM -0700, Easwar Hariharan wrote:
From: Easwar Hariharan
Needs a commit log, even if it is nothing more than the subject line.
Also read the top of include/linux/pci_ids.h, because it looks like
some of these are only used in one driver and hence do not need to be
in pci_ids.h.
Thanks, this was a separate patch for exactly that reason instead of being
combined with the patch to move the vendor ID. I sent it anyway because
of other device IDs in the pci_ids.h file used only by a single driver,
which I
assume are legacy artifacts. After removing the MANA device IDs, the patch
boils down to a trivial rename of the HYPERV_VIDEO device ID, so I'm just
dropping this patch.
- Easwar
Signed-off-by: Easwar Hariharan
---
drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 2 +-
drivers/net/ethernet/microsoft/mana/gdma.h | 3 ---
drivers/net/ethernet/microsoft/mana/gdma_main.c | 6 +++---
drivers/video/fbdev/hyperv_fb.c | 4 ++--
include/linux/pci_ids.h | 4 +++-
5 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index f84d397..24c2def 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -51,7 +51,7 @@ static void hyperv_pci_remove(struct pci_dev *pdev)
static const struct pci_device_id hyperv_pci_tbl[] = {
{
.vendor = PCI_VENDOR_ID_MICROSOFT,
- .device = PCI_DEVICE_ID_HYPERV_VIDEO,
+ .device = PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO,
},
{ /* end of list */ }
};
diff --git a/drivers/net/ethernet/microsoft/mana/gdma.h
b/drivers/net/ethernet/microsoft/mana/gdma.h
index 4a6efe6..9d3a9f7 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma.h
+++ b/drivers/net/ethernet/microsoft/mana/gdma.h
@@ -476,9 +476,6 @@ struct gdma_eqe {
#define GDMA_SRIOV_REG_CFG_BASE_OFF 0x108
-#define MANA_PF_DEVICE_ID 0x00B9
-#define MANA_VF_DEVICE_ID 0x00BA
-
struct gdma_posted_wqe_info {
u32 wqe_size_in_bu;
};
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 00d8198..18cf168 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1333,7 +1333,7 @@ static void mana_gd_cleanup(struct pci_dev *pdev)
static bool mana_is_pf(unsigned short dev_id)
{
- return dev_id == MANA_PF_DEVICE_ID;
+ return dev_id == PCI_DEVICE_ID_MICROSOFT_MANA_PF;
}
static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
@@ -1466,8 +1466,8 @@ static void mana_gd_shutdown(struct pci_dev *pdev)
}
static const struct pci_device_id mana_id_table[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_PF_DEVICE_ID) },
- { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_VF_DEVICE_ID) },
+ { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_MICROSOFT_MANA_PF)
},
+ { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_MICROSOFT_MANA_VF)
},
{ }
};
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index b58b445..118e244 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -997,7 +997,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct
fb_info *info)
if (!gen2vm) {
pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
- PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
+ PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO, NULL);
if (!pdev) {
pr_err("Unable to find PCI Hyper-V video\n");
return -ENODEV;
@@ -1311,7 +1311,7 @@ static int hvfb_resume(struct hv_device *hdev)
static const struct pci_device_id pci_stub_id_table[] = {
{
.vendor = PCI_VENDOR_ID_MICROSOFT,
- .device = PCI_DEVICE_ID_HYPERV_VIDEO,
+ .device = PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO,
},
{ /* end of list */ }
};
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 15b49e6..fe3517f 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2080,7 +2080,9 @@
#define PCI_DEVICE_ID_VT1724 0x1724
#define PCI_VENDOR_ID_MICROSOFT 0x1414
-#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
+#define PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO 0x5353
+#define PCI_DEVICE_ID_MICROSOFT_MANA_PF0x00B9
+#define PCI_DEVICE_ID_MICROSOFT_MANA_VF0x00BA
#define PCI_VENDOR_ID_OXSEMI 0x1415
#define PCI_DEVICE_ID_OXSEMI_12PCI840 0x8403
--
1.8.3.1
Re: [PATCH v2 2/2] pci_ids: Add the various Microsoft PCI device IDs
On 9/9/22 19:38, Bjorn Helgaas wrote:
Please follow the PCI subject line conventions. Discover it with
"git log --oneline include/linux/pci_ids.h".
Thanks, updated.
On Fri, Sep 09, 2022 at 11:50:25AM -0700, Easwar Hariharan wrote:
From: Easwar Hariharan
Needs a commit log, even if it is nothing more than the subject line.
Also read the top of include/linux/pci_ids.h, because it looks like
some of these are only used in one driver and hence do not need to be
in pci_ids.h.
Thanks, this was a separate patch for exactly that reason instead of being
combined with the patch to move the vendor ID. I assume other device IDs
in the pci_ids.h file used only by a single driver are legacy artifacts.
After
removing the MANA device IDs, the patch boils down to a trivial rename
of the HYPERV_VIDEO device ID, so I'm just dropping this patch.
- Easwar
Signed-off-by: Easwar Hariharan
---
drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 2 +-
drivers/net/ethernet/microsoft/mana/gdma.h | 3 ---
drivers/net/ethernet/microsoft/mana/gdma_main.c | 6 +++---
drivers/video/fbdev/hyperv_fb.c | 4 ++--
include/linux/pci_ids.h | 4 +++-
5 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index f84d397..24c2def 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -51,7 +51,7 @@ static void hyperv_pci_remove(struct pci_dev *pdev)
static const struct pci_device_id hyperv_pci_tbl[] = {
{
.vendor = PCI_VENDOR_ID_MICROSOFT,
- .device = PCI_DEVICE_ID_HYPERV_VIDEO,
+ .device = PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO,
},
{ /* end of list */ }
};
diff --git a/drivers/net/ethernet/microsoft/mana/gdma.h
b/drivers/net/ethernet/microsoft/mana/gdma.h
index 4a6efe6..9d3a9f7 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma.h
+++ b/drivers/net/ethernet/microsoft/mana/gdma.h
@@ -476,9 +476,6 @@ struct gdma_eqe {
#define GDMA_SRIOV_REG_CFG_BASE_OFF 0x108
-#define MANA_PF_DEVICE_ID 0x00B9
-#define MANA_VF_DEVICE_ID 0x00BA
-
struct gdma_posted_wqe_info {
u32 wqe_size_in_bu;
};
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 00d8198..18cf168 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1333,7 +1333,7 @@ static void mana_gd_cleanup(struct pci_dev *pdev)
static bool mana_is_pf(unsigned short dev_id)
{
- return dev_id == MANA_PF_DEVICE_ID;
+ return dev_id == PCI_DEVICE_ID_MICROSOFT_MANA_PF;
}
static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
@@ -1466,8 +1466,8 @@ static void mana_gd_shutdown(struct pci_dev *pdev)
}
static const struct pci_device_id mana_id_table[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_PF_DEVICE_ID) },
- { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_VF_DEVICE_ID) },
+ { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_MICROSOFT_MANA_PF)
},
+ { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_MICROSOFT_MANA_VF)
},
{ }
};
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index b58b445..118e244 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -997,7 +997,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct
fb_info *info)
if (!gen2vm) {
pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
- PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
+ PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO, NULL);
if (!pdev) {
pr_err("Unable to find PCI Hyper-V video\n");
return -ENODEV;
@@ -1311,7 +1311,7 @@ static int hvfb_resume(struct hv_device *hdev)
static const struct pci_device_id pci_stub_id_table[] = {
{
.vendor = PCI_VENDOR_ID_MICROSOFT,
- .device = PCI_DEVICE_ID_HYPERV_VIDEO,
+ .device = PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO,
},
{ /* end of list */ }
};
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 15b49e6..fe3517f 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2080,7 +2080,9 @@
#define PCI_DEVICE_ID_VT1724 0x1724
#define PCI_VENDOR_ID_MICROSOFT 0x1414
-#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
+#define PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO 0x5353
+#define PCI_DEVICE_ID_MICROSOFT_MANA_PF0x00B9
+#define PCI_DEVICE_ID_MICROSOFT_MANA_VF0x00BA
#define PCI_VENDOR_ID_OXSEMI 0x1415
#define PCI_DEVICE_ID_OXSEMI_12PCI840 0x8403
--
1.8.3.1
[linux-next:master] BUILD REGRESSION 9a82ccda91ed2b40619cb3c10d446ae1f97bab6e
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 9a82ccda91ed2b40619cb3c10d446ae1f97bab6e Add linux-next specific files for 20220909 Error/Warning reports: https://lore.kernel.org/linux-mm/202209042337.fqi69rlv-...@intel.com https://lore.kernel.org/linux-mm/202209080718.y5qmlnkh-...@intel.com Error/Warning: (recently discovered and may have been fixed) ./drivers/gpu/drm/drm_atomic_helper.c:802: warning: expecting prototype for drm_atomic_helper_check_wb_connector_state(). Prototype was for drm_atomic_helper_check_wb_encoder_state() instead ERROR: modpost: "__divdi3" [drivers/gpu/drm/vkms/vkms.ko] undefined! ERROR: modpost: "__udivdi3" [drivers/gpu/drm/vkms/vkms.ko] undefined! arm-linux-gnueabi-ld: vkms_formats.c:(.text+0x824): undefined reference to `__aeabi_ldivmod' drivers/base/regmap/regmap-mmio.c:222:17: error: implicit declaration of function 'writesb'; did you mean 'writeb'? [-Werror=implicit-function-declaration] drivers/base/regmap/regmap-mmio.c:225:17: error: implicit declaration of function 'writesw'; did you mean 'writew'? [-Werror=implicit-function-declaration] drivers/base/regmap/regmap-mmio.c:228:17: error: implicit declaration of function 'writesl'; did you mean 'writel'? [-Werror=implicit-function-declaration] drivers/base/regmap/regmap-mmio.c:358:17: error: implicit declaration of function 'readsb'; did you mean 'readb'? [-Werror=implicit-function-declaration] drivers/base/regmap/regmap-mmio.c:361:17: error: implicit declaration of function 'readsw'; did you mean 'readw'? [-Werror=implicit-function-declaration] drivers/base/regmap/regmap-mmio.c:364:17: error: implicit declaration of function 'readsl'; did you mean 'readl'? [-Werror=implicit-function-declaration] drivers/gpu/drm/amd/amdgpu/imu_v11_0_3.c:139:6: warning: no previous prototype for 'imu_v11_0_3_program_rlc_ram' [-Wmissing-prototypes] drivers/gpu/drm/drm_atomic_helper.c:802: warning: expecting prototype for drm_atomic_helper_check_wb_connector_state(). Prototype was for drm_atomic_helper_check_wb_encoder_state() instead drivers/gpu/drm/vkms/vkms_formats.c:259: undefined reference to `__divdi3' drivers/gpu/drm/vkms/vkms_plane.c:110 vkms_plane_atomic_update() warn: variable dereferenced before check 'fb' (see line 108) drivers/scsi/qla2xxx/qla_os.c:2854:23: warning: assignment to 'struct trace_array *' from 'int' makes pointer from integer without a cast [-Wint-conversion] drivers/scsi/qla2xxx/qla_os.c:2854:25: error: implicit declaration of function 'trace_array_get_by_name'; did you mean 'trace_array_set_clr_event'? [-Werror=implicit-function-declaration] drivers/scsi/qla2xxx/qla_os.c:2869:9: error: implicit declaration of function 'trace_array_put' [-Werror=implicit-function-declaration] kernel/bpf/memalloc.c:499 bpf_mem_alloc_destroy() error: potentially dereferencing uninitialized 'c'. ld: drivers/gpu/drm/vkms/vkms_formats.c:260: undefined reference to `__divdi3' ld: vkms_formats.c:(.text+0x362): undefined reference to `__divdi3' ld: vkms_formats.c:(.text+0x3b2): undefined reference to `__divdi3' ld: vkms_formats.c:(.text+0x3ba): undefined reference to `__divdi3' ld: vkms_formats.c:(.text+0x47f): undefined reference to `__divdi3' microblaze-linux-ld: drivers/gpu/drm/vkms/vkms_formats.c:260: undefined reference to `__divdi3' mips-linux-ld: vkms_formats.c:(.text+0x8b8): undefined reference to `__divdi3' mips-linux-ld: vkms_formats.c:(.text.argb_u16_to_RGB565+0xd0): undefined reference to `__divdi3' sound/soc/codecs/tas2562.c:442:13: warning: variable 'ret' set but not used [-Wunused-but-set-variable] vkms_formats.c:(.text+0x266): undefined reference to `__divdi3' vkms_formats.c:(.text+0x338): undefined reference to `__divdi3' vkms_formats.c:(.text+0x388): undefined reference to `__divdi3' vkms_formats.c:(.text+0x390): undefined reference to `__divdi3' vkms_formats.c:(.text+0x455): undefined reference to `__divdi3' vkms_formats.c:(.text+0x804): undefined reference to `__aeabi_ldivmod' vkms_formats.c:(.text+0x89c): undefined reference to `__divdi3' vkms_formats.c:(.text.argb_u16_to_RGB565+0xb0): undefined reference to `__divdi3' Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-imu_v11_0_3.c:warning:no-previous-prototype-for-imu_v11_0_3_program_rlc_ram | |-- drivers-gpu-drm-drm_atomic_helper.c:warning:expecting-prototype-for-drm_atomic_helper_check_wb_connector_state().-Prototype-was-for-drm_atomic_helper_check_wb_encoder_state()-instead | |-- drivers-scsi-qla2xxx-qla_os.c:error:implicit-declaration-of-function-trace_array_get_by_name | |-- drivers-scsi-qla2xxx-qla_os.c:error:implicit-declaration-of-function-trace_array_put | |-- drivers-scsi-qla2xxx-qla_os.c:warning:assignment-to-struct-trace_array-from-int-makes-pointer-from-integer-without-a-cast | `-- sound-soc-codecs-tas2562.c:warning:variable-ret-set-but-not-used |-- arc-allyesconfig |
Re: [PATCH v4 12/15] drm/i915/huc: stall media submission until HuC is loaded
On 9/8/2022 5:16 PM, Daniele Ceraolo Spurio wrote:
Wait on the fence to be signalled to avoid the submissions finding HuC
not yet loaded.
Signed-off-by: Daniele Ceraolo Spurio
Cc: Tony Ye
Reviewed-by: Alan Previn
---
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 6 ++
drivers/gpu/drm/i915/i915_request.c| 24
2 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 915d281c1c72..52db03620c60 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -81,6 +81,12 @@ static inline bool intel_huc_is_loaded_by_gsc(const struct
intel_huc *huc)
return huc->fw.loaded_via_gsc;
}
+static inline bool intel_huc_wait_required(struct intel_huc *huc)
+{
+ return intel_huc_is_used(huc) && intel_huc_is_loaded_by_gsc(huc) &&
+ !intel_huc_is_authenticated(huc);
+}
+
void intel_huc_load_status(struct intel_huc *huc, struct drm_printer *p);
#endif
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 62fad16a55e8..77f45a3cb01f 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1621,6 +1621,20 @@ i915_request_await_object(struct i915_request *to,
return ret;
}
+static void i915_request_await_huc(struct i915_request *rq)
+{
+ struct intel_huc *huc = >context->engine->gt->uc.huc;
+
+ /* don't stall kernel submissions! */
+ if (!rcu_access_pointer(rq->context->gem_context))
+ return;
+
+ if (intel_huc_wait_required(huc))
+ i915_sw_fence_await_sw_fence(>submit,
+>delayed_load.fence,
+>submitq);
+}
+
static struct i915_request *
__i915_request_ensure_parallel_ordering(struct i915_request *rq,
struct intel_timeline *timeline)
@@ -1702,6 +1716,16 @@ __i915_request_add_to_timeline(struct i915_request *rq)
struct intel_timeline *timeline = i915_request_timeline(rq);
struct i915_request *prev;
+ /*
+* Media workloads may require HuC, so stall them until HuC loading is
+* complete. Note that HuC not being loaded when a user submission
+* arrives can only happen when HuC is loaded via GSC and in that case
+* we still expect the window between us starting to accept submissions
+* and HuC loading completion to be small (a few hundred ms).
+*/
+ if (rq->engine->class == VIDEO_DECODE_CLASS)
+ i915_request_await_huc(rq);
+
Acked-by: Tony Ye
Thanks,
Tony
/*
* Dependency tracking and request ordering along the timeline
* is special cased so that we can eliminate redundant ordering
Re: [PATCH v4 14/15] drm/i915/huc: define gsc-compatible HuC fw for DG2
On 9/8/2022 5:16 PM, Daniele Ceraolo Spurio wrote:
The fw name is different and we need to record the fact that the blob is
gsc-loaded, so add a new macro to help.
Note: A-step DG2 G10 does not support HuC loading via GSC and would
require a separate firmware to be loaded the legacy way, but that's
not a production stepping so we're not going to bother.
v2: rebase on new fw fetch logic
Signed-off-by: Daniele Ceraolo Spurio
Cc: Tony Ye
Reviewed-by: Alan Previn #v1
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 4792960d9c04..09e06ac8bcf1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -91,7 +91,8 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
fw_def(BROXTON, 0, guc_mmp(bxt, 70, 1, 1)) \
fw_def(SKYLAKE, 0, guc_mmp(skl, 70, 1, 1))
-#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp) \
+#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp, huc_gsc) \
+ fw_def(DG2, 0, huc_gsc(dg2)) \
fw_def(ALDERLAKE_P, 0, huc_mmp(tgl, 7, 9, 3)) \
fw_def(ALDERLAKE_S, 0, huc_mmp(tgl, 7, 9, 3)) \
fw_def(DG1, 0, huc_mmp(dg1, 7, 9, 3)) \
@@ -137,6 +138,9 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
#define MAKE_HUC_FW_PATH_BLANK(prefix_) \
__MAKE_UC_FW_PATH_BLANK(prefix_, "_huc")
+#define MAKE_HUC_FW_PATH_GSC(prefix_) \
+ __MAKE_UC_FW_PATH_BLANK(prefix_, "_huc_gsc")
+
#define MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \
__MAKE_UC_FW_PATH_MMP(prefix_, "_huc_", major_, minor_, patch_)
@@ -149,7 +153,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
MODULE_FIRMWARE(uc_);
INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH_MAJOR, MAKE_GUC_FW_PATH_MMP)
-INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK,
MAKE_HUC_FW_PATH_MMP)
+INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK,
MAKE_HUC_FW_PATH_MMP, MAKE_HUC_FW_PATH_GSC)
/*
* The next expansion of the table macros (in __uc_fw_auto_select below)
provides
@@ -164,6 +168,7 @@ struct __packed uc_fw_blob {
u8 major;
u8 minor;
u8 patch;
+ bool loaded_via_gsc;
};
#define UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
@@ -172,16 +177,16 @@ struct __packed uc_fw_blob {
.patch = patch_, \
.path = path_,
-#define UC_FW_BLOB_NEW(major_, minor_, patch_, path_) \
+#define UC_FW_BLOB_NEW(major_, minor_, patch_, gsc_, path_) \
{ UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
- .legacy = false }
+ .legacy = false, .loaded_via_gsc = gsc_ }
#define UC_FW_BLOB_OLD(major_, minor_, patch_, path_) \
{ UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
.legacy = true }
#define GUC_FW_BLOB(prefix_, major_, minor_) \
- UC_FW_BLOB_NEW(major_, minor_, 0, \
+ UC_FW_BLOB_NEW(major_, minor_, 0, false, \
MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_))
#define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \
@@ -189,12 +194,15 @@ struct __packed uc_fw_blob {
MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_))
#define HUC_FW_BLOB(prefix_) \
- UC_FW_BLOB_NEW(0, 0, 0, MAKE_HUC_FW_PATH_BLANK(prefix_))
+ UC_FW_BLOB_NEW(0, 0, 0, false, MAKE_HUC_FW_PATH_BLANK(prefix_))
#define HUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \
UC_FW_BLOB_OLD(major_, minor_, patch_, \
MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_))
+#define HUC_FW_BLOB_GSC(prefix_) \
+ UC_FW_BLOB_NEW(0, 0, 0, true, MAKE_HUC_FW_PATH_GSC(prefix_))
+
struct __packed uc_fw_platform_requirement {
enum intel_platform p;
u8 rev; /* first platform rev using this FW */
@@ -220,7 +228,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct
intel_uc_fw *uc_fw)
INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
GUC_FW_BLOB_MMP)
};
static const struct uc_fw_platform_requirement blobs_huc[] = {
- INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
HUC_FW_BLOB_MMP)
+ INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
};
static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] =
{
[INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
@@ -266,6 +274,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct
intel_uc_fw *uc_fw)
uc_fw->file_wanted.path = blob->path;
uc_fw->file_wanted.major_ver = blob->major;
uc_fw->file_wanted.minor_ver = blob->minor;
+ uc_fw->loaded_via_gsc =
Re: [PATCH v4 11/15] drm/i915/huc: track delayed HuC load with a fence
Reviewed-by: Alan Previn
On Thu, 2022-09-08 at 17:16 -0700, Ceraolo Spurio, Daniele wrote:
> Given that HuC load is delayed on DG2, this patch adds support for a fence
> that can be used to wait for load completion. No waiters are added in this
> patch (they're coming up in the next one), to keep the focus of the
> patch on the tracking logic.
>
> The full HuC loading flow on boot DG2 is as follows:
> 1) i915 exports the GSC as an aux device;
> 2) the mei-gsc driver is loaded on the aux device;
> 3) the mei-pxp component is loaded;
> 4) mei-pxp calls back into i915 and we load the HuC.
>
> Between steps 1 and 2 there can be several seconds of gap, mainly due to
> the kernel doing other work during the boot.
> The resume flow is slightly different, because we don't need to
> re-expose or re-probe the aux device, so we go directly to step 3 once
> i915 and mei-gsc have completed their resume flow.
>
> Here's an example of the boot timing, captured with some logs added to
> i915:
>
> [ 17.908307] [drm] adding GSC device
> [ 17.915717] [drm] i915 probe done
> [ 22.282917] [drm] mei-gsc bound
> [ 22.938153] [drm] HuC authenticated
>
> Also to note is that if something goes wrong during GSC HW init the
> mei-gsc driver will still bind, but steps 3 and 4 will not happen.
>
> The status tracking is done by registering a bus_notifier to receive a
> callback when the mei-gsc driver binds, with a large enough timeout to
> account for delays. Once mei-gsc is bound, we switch to a smaller
> timeout to wait for the mei-pxp component to load.
> The fence is signalled on HuC load complete or if anything goes wrong in
> any of the tracking steps. Timeout are enforced via hrtimer callbacks.
>
> v2: fix includes (Jani)
>
> Signed-off-by: Daniele Ceraolo Spurio
> Reviewed-by: Alan Previn #v1
> ---
> drivers/gpu/drm/i915/gt/intel_gsc.c| 22 ++-
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 199 +
> drivers/gpu/drm/i915/gt/uc/intel_huc.h | 23 +++
> 3 files changed, 241 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c
> b/drivers/gpu/drm/i915/gt/intel_gsc.c
> index 7af6db3194dd..f544f70401f8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gsc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
> @@ -142,8 +142,14 @@ static void gsc_destroy_one(struct drm_i915_private
> *i915,
> struct intel_gsc_intf *intf = >intf[intf_id];
>
> if (intf->adev) {
> - auxiliary_device_delete(>adev->aux_dev);
> - auxiliary_device_uninit(>adev->aux_dev);
> + struct auxiliary_device *aux_dev = >adev->aux_dev;
> +
> + if (intf_id == 0)
> +
> intel_huc_unregister_gsc_notifier(_to_gt(gsc)->uc.huc,
> + aux_dev->dev.bus);
> +
> + auxiliary_device_delete(aux_dev);
> + auxiliary_device_uninit(aux_dev);
> intf->adev = NULL;
> }
>
> @@ -242,14 +248,24 @@ static void gsc_init_one(struct drm_i915_private *i915,
> struct intel_gsc *gsc,
> goto fail;
> }
>
> + intf->adev = adev; /* needed by the notifier */
> +
> + if (intf_id == 0)
> + intel_huc_register_gsc_notifier(_to_gt(gsc)->uc.huc,
> + aux_dev->dev.bus);
> +
> ret = auxiliary_device_add(aux_dev);
> if (ret < 0) {
> drm_err(>drm, "gsc aux add failed %d\n", ret);
> + if (intf_id == 0)
> +
> intel_huc_unregister_gsc_notifier(_to_gt(gsc)->uc.huc,
> + aux_dev->dev.bus);
> + intf->adev = NULL;
> +
> /* adev will be freed with the put_device() and .release
> sequence */
> auxiliary_device_uninit(aux_dev);
> goto fail;
> }
> - intf->adev = adev;
>
> return;
> fail:
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index f0188931d8e4..13d93e69766f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -10,6 +10,9 @@
> #include "intel_huc.h"
> #include "i915_drv.h"
>
> +#include
> +#include
> +
> /**
> * DOC: HuC
> *
> @@ -42,6 +45,164 @@
> * HuC-specific commands.
> */
>
> +/*
> + * MEI-GSC load is an async process. The probing of the exposed aux device
> + * (see intel_gsc.c) usually happens a few seconds after i915 probe,
> depending
> + * on when the kernel schedules it. Unless something goes terribly wrong,
> we're
> + * guaranteed for this to happen during boot, so the big timeout is a safety
> net
> + * that we never expect to need.
> + * MEI-PXP + HuC load usually takes ~300ms, but if the GSC needs to be
> resumed
> + * and/or reset, this can take longer.
> + */
> +#define GSC_INIT_TIMEOUT_MS 1
> +#define PXP_INIT_TIMEOUT_MS 2000
> +
> +static int
Re: [PATCH] drm/amdgpu/display: remove unneeded "default n" options
Applied. Thanks, Alex On Fri, Sep 9, 2022 at 3:54 PM Jingyu Wang wrote: > > Remove "default n" options. If the "default" line is removed, it > defaults to 'n'. > > Signed-off-by: Jingyu Wang > --- > drivers/gpu/drm/amd/display/Kconfig | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/Kconfig > b/drivers/gpu/drm/amd/display/Kconfig > index 413d8c6d592f..6925e0280dbe 100644 > --- a/drivers/gpu/drm/amd/display/Kconfig > +++ b/drivers/gpu/drm/amd/display/Kconfig > @@ -28,7 +28,6 @@ config DRM_AMD_DC_SI > bool "AMD DC support for Southern Islands ASICs" > depends on DRM_AMDGPU_SI > depends on DRM_AMD_DC > - default n > help > Choose this option to enable new AMD DC support for SI asics > by default. This includes Tahiti, Pitcairn, Cape Verde, Oland. > @@ -43,7 +42,6 @@ config DEBUG_KERNEL_DC > > config DRM_AMD_SECURE_DISPLAY > bool "Enable secure display support" > -default n > depends on DEBUG_FS > depends on DRM_AMD_DC_DCN > help > > base-commit: 5957ac6635a1a12d4aa2661bbf04d3085a73372a > -- > 2.34.1 >
Re: [PATCH] drm/amd/display: Remove var declaration in dcn32_full_validate_bw_helper()
Already fixed a few days ago. Thanks for the patch.
Alex
On Thu, Sep 8, 2022 at 4:51 AM Jocelyn Falempe wrote:
>
> The variable i is already declared as uint32_t in the same function.
>
> This fixes the following error, when compiling this code on older kernel:
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c: In function
> 'dcn32_full_validate_bw_helper':
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:1018:3: error:
> 'for' loop initial declarations are only allowed in C99 or C11 mode
>for (int i = 0; i < dc->res_pool->pipe_count; i++) {
>^~~
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:1018:3: note:
> use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:982:11: error:
> unused variable 'i' [-Werror=unused-variable]
> uint32_t i = 0;
>
> Fixes: f5b9c1ffabce ("drm/amd/display: Re-initialize viewport after pipe
> merge")
>
> Signed-off-by: Jocelyn Falempe
> ---
> drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> index 8e4c9d0887ce..56f02b1ea808 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c
> @@ -1015,7 +1015,7 @@ static void dcn32_full_validate_bw_helper(struct dc *dc,
>
> dcn32_merge_pipes_for_subvp(dc, context);
> // to re-initialize viewport after the pipe merge
> - for (int i = 0; i < dc->res_pool->pipe_count; i++) {
> + for (i = 0; i < dc->res_pool->pipe_count; i++) {
> struct pipe_ctx *pipe_ctx =
> >res_ctx.pipe_ctx[i];
>
> if (!pipe_ctx->plane_state || !pipe_ctx->stream)
> --
> 2.37.3
>
Re: [PATCH] drm/amd/display: fix repeated words in comments
Applied. Thanks! On Wed, Sep 7, 2022 at 12:11 AM Jilin Yuan wrote: > > Delete the redundant word 'in'. > > Signed-off-by: Jilin Yuan > --- > drivers/gpu/drm/amd/display/dc/dce/dce_audio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c > b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c > index bdb6bac8dd97..c94a966c6612 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c > @@ -300,7 +300,7 @@ static void set_high_bit_rate_capable( > AZ_REG_WRITE(AZALIA_F0_CODEC_PIN_CONTROL_RESPONSE_HBR, value); > } > > -/* set video latency in in ms/2+1 */ > +/* set video latency in ms/2+1 */ > static void set_video_latency( > struct audio *audio, > int latency_in_ms) > -- > 2.36.1 >
Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_amdkfd_gpuvm.c
Applied. Thanks!
Alex
On Tue, Sep 6, 2022 at 2:48 PM Deucher, Alexander
wrote:
>
> [Public]
>
>
> Yeah, seems to be a mix. I don't have a strong opinion as long as it has MIT.
>
> Alex
>
>
> From: Kuehling, Felix
> Sent: Tuesday, September 6, 2022 9:46 AM
> To: Jingyu Wang ; Deucher, Alexander
> ; Koenig, Christian ;
> Pan, Xinhui ; airl...@linux.ie ;
> dan...@ffwll.ch
> Cc: amd-...@lists.freedesktop.org ;
> dri-devel@lists.freedesktop.org ;
> linux-ker...@vger.kernel.org
> Subject: Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_amdkfd_gpuvm.c
>
>
> Am 2022-09-05 um 04:38 schrieb Jingyu Wang:
> > Fix everything checkpatch.pl complained about in amdgpu_amdkfd_gpuvm.c
> >
> > Signed-off-by: Jingyu Wang
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index cbd593f7d553..eff596c60c89 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -1,3 +1,4 @@
> > +// SPDX-License-Identifier: MIT
>
> I'm not sure if this is correct. We've used "GPL-2.0 OR MIT" in KFD. In
> amdgpu there is currently a mix of licenses. Alex, do you want to make a
> call on a consistent one to use in amdgpu?
>
> Other than that, this patch is
>
> Reviewed-by: Felix Kuehling
>
>
> > /*
> >* Copyright 2014-2018 Advanced Micro Devices, Inc.
> >*
> > @@ -1612,6 +1613,7 @@ size_t amdgpu_amdkfd_get_available_memory(struct
> > amdgpu_device *adev)
> >uint64_t reserved_for_pt =
> >ESTIMATE_PT_SIZE(amdgpu_amdkfd_total_mem_size);
> >size_t available;
> > +
> >spin_lock(_mem_limit.mem_limit_lock);
> >available = adev->gmc.real_vram_size
> >- adev->kfd.vram_used_aligned
> > @@ -2216,7 +2218,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct
> > amdgpu_device *adev,
> > {
> >if (atomic_read(>gmc.vm_fault_info_updated) == 1) {
> >*mem = *adev->gmc.vm_fault_info;
> > - mb();
> > + mb(); /* make sure read happened */
> >atomic_set(>gmc.vm_fault_info_updated, 0);
> >}
> >return 0;
> >
> > base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
RE: [PATCH v3 07/14] drm/i915: Use a DRM-managed action to release the PCI bridge device
> -Original Message-
> From: dri-devel On Behalf Of Matt
> Roper
> Sent: Tuesday, September 6, 2022 4:49 PM
> To: intel-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Subject: [PATCH v3 07/14] drm/i915: Use a DRM-managed action to release the
> PCI bridge device
>
> As we start supporting multiple uncore structures in future patches, the
> MMIO cleanup (which make also get called mid-init if there's a failure)
> will become more complicated. Moving to DRM-managed actions will help
> keep things simple.
>
LGTM,
Reviewed-by: Radhakrishna Sripada
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/i915/i915_driver.c | 12 +---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c
> b/drivers/gpu/drm/i915/i915_driver.c
> index 18acba1bc3b0..1f46dd1ffaf7 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -105,6 +105,12 @@ static const char irst_name[] = "INT3392";
>
> static const struct drm_driver i915_drm_driver;
>
> +static void i915_release_bridge_dev(struct drm_device *dev,
> + void *bridge)
> +{
> + pci_dev_put(bridge);
> +}
> +
> static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
> {
> int domain = pci_domain_nr(to_pci_dev(dev_priv->drm.dev)->bus);
> @@ -115,7 +121,9 @@ static int i915_get_bridge_dev(struct drm_i915_private
> *dev_priv)
> drm_err(_priv->drm, "bridge device not found\n");
> return -EIO;
> }
> - return 0;
> +
> + return drmm_add_action_or_reset(_priv->drm,
> i915_release_bridge_dev,
> + dev_priv->bridge_dev);
> }
>
> /* Allocate space for the MCH regs if needed, return nonzero on error */
> @@ -452,7 +460,6 @@ static int i915_driver_mmio_probe(struct
> drm_i915_private *dev_priv)
> err_uncore:
> intel_teardown_mchbar(dev_priv);
> intel_uncore_fini_mmio(_priv->uncore);
> - pci_dev_put(dev_priv->bridge_dev);
>
> return ret;
> }
> @@ -465,7 +472,6 @@ static void i915_driver_mmio_release(struct
> drm_i915_private *dev_priv)
> {
> intel_teardown_mchbar(dev_priv);
> intel_uncore_fini_mmio(_priv->uncore);
> - pci_dev_put(dev_priv->bridge_dev);
> }
>
> /**
> --
> 2.37.2
Re: [PATCH v3 5/6] drm/sched: Use parent fence instead of finished
Got it.
Reviewed-by: Andrey Grodzovsky
Andrey
On 2022-09-09 16:30, Yadav, Arvind wrote:
On 9/9/2022 11:02 PM, Andrey Grodzovsky wrote:
What exactly is the scenario which this patch fixes in more detail
please ?
GPU reset issue started after adding [PATCH 6/6].
Root cause -> In drm_sched_get_cleanup_job(), We use the finished
fence status bit to check the job status dma_fence_is_signaled(). If a
job is signaled (DMA_FENCE_FLAG_SIGNALED_BIT is set), then we cancel
the reset worker thread.
After applying [patch 6] now we are checking enable signaling in
dma_fence_is_signaled() by checking DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT
bit. but signaling is not enabled for the finished fence. As a result,
dma_fence_is_signaled() always returns false, and
drm_sched_get_cleanup_job() will not cancel the reset worker thread,
resulting in the GPU reset.
To Fix the above issue Christian suggested that we can use
parent(hardware) fence instead of finished fence because signaling
enabled by the calling of dma_fence_add_callback() for parent fence.
As a result, dma_fence_is_signaled() will return the correct fence
status and reset worker thread can be cancelled in
drm_sched_get_cleanup_job().
~arvind
Andrey
On 2022-09-09 13:08, Arvind Yadav wrote:
Using the parent fence instead of the finished fence
to get the job status. This change is to avoid GPU
scheduler timeout error which can cause GPU reset.
Signed-off-by: Arvind Yadav
---
changes in v1,v2 - Enable signaling for finished fence in sche_main()
is removed
---
drivers/gpu/drm/scheduler/sched_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index e0ab14e0fb6b..2ac28ad11432 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -829,7 +829,7 @@ drm_sched_get_cleanup_job(struct
drm_gpu_scheduler *sched)
job = list_first_entry_or_null(>pending_list,
struct drm_sched_job, list);
- if (job && dma_fence_is_signaled(>s_fence->finished)) {
+ if (job && dma_fence_is_signaled(job->s_fence->parent)) {
/* remove job from pending_list */
list_del_init(>list);
@@ -841,7 +841,7 @@ drm_sched_get_cleanup_job(struct
drm_gpu_scheduler *sched)
if (next) {
next->s_fence->scheduled.timestamp =
- job->s_fence->finished.timestamp;
+ job->s_fence->parent->timestamp;
/* start TO timer for next job */
drm_sched_start_timeout(sched);
}
Re: [PATCH v4 10/15] drm/i915/dg2: setup HuC loading via GSC
Nearly identical as before:
Reviewed-by: Alan Previn
On Thu, 2022-09-08 at 17:16 -0700, Ceraolo Spurio, Daniele wrote:
> The GSC will perform both the load and the authentication, so we just
> need to check the auth bit after the GSC has replied.
> Since we require the PXP module to load the HuC, the earliest we can
> trigger the load is during the pxp_bind operation.
>
> Note that GSC-loaded HuC survives GT reset, so we need to just mark it
> as ready when we re-init the GT HW.
>
> V2: move setting of HuC fw error state to the failure path of the HuC
> auth function, so it covers both the legacy and new auth flows
> V4:
> 1. Fix typo in the commit message
> 2. style fix in intel_huc_wait_for_auth_complete()
>
> Signed-off-by: Daniele Ceraolo Spurio
> Signed-off-by: Vitaly Lubart
> Signed-off-by: Tomas Winkler
> Reviewed-by: Alan Previn #v2
> ---
> drivers/gpu/drm/i915/gt/uc/intel_huc.c| 41 +++
> drivers/gpu/drm/i915/gt/uc/intel_huc.h| 2 ++
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 34 +++
> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.h | 1 +
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 14 +++-
> 5 files changed, 77 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 3bb8838e325a..f0188931d8e4 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -125,6 +125,28 @@ void intel_huc_fini(struct intel_huc *huc)
> intel_uc_fw_fini(>fw);
> }
>
> +int intel_huc_wait_for_auth_complete(struct intel_huc *huc)
> +{
> + struct intel_gt *gt = huc_to_gt(huc);
> + int ret;
> +
> + ret = __intel_wait_for_register(gt->uncore,
> + huc->status.reg,
> + huc->status.mask,
> + huc->status.value,
> + 2, 50, NULL);
> +
> + if (ret) {
> + drm_err(>i915->drm, "HuC: Firmware not verified %d\n", ret);
> + intel_uc_fw_change_status(>fw,
> INTEL_UC_FIRMWARE_LOAD_FAIL);
> + return ret;
> + }
> +
> + intel_uc_fw_change_status(>fw, INTEL_UC_FIRMWARE_RUNNING);
> + drm_info(>i915->drm, "HuC authenticated\n");
> + return 0;
> +}
> +
> /**
> * intel_huc_auth() - Authenticate HuC uCode
> * @huc: intel_huc structure
> @@ -161,27 +183,18 @@ int intel_huc_auth(struct intel_huc *huc)
> }
>
> /* Check authentication status, it should be done by now */
> - ret = __intel_wait_for_register(gt->uncore,
> - huc->status.reg,
> - huc->status.mask,
> - huc->status.value,
> - 2, 50, NULL);
> - if (ret) {
> - DRM_ERROR("HuC: Firmware not verified %d\n", ret);
> + ret = intel_huc_wait_for_auth_complete(huc);
> + if (ret)
> goto fail;
> - }
>
> - intel_uc_fw_change_status(>fw, INTEL_UC_FIRMWARE_RUNNING);
> - drm_info(>i915->drm, "HuC authenticated\n");
> return 0;
>
> fail:
> i915_probe_error(gt->i915, "HuC: Authentication failed %d\n", ret);
> - intel_uc_fw_change_status(>fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
> return ret;
> }
>
> -static bool huc_is_authenticated(struct intel_huc *huc)
> +bool intel_huc_is_authenticated(struct intel_huc *huc)
> {
> struct intel_gt *gt = huc_to_gt(huc);
> intel_wakeref_t wakeref;
> @@ -223,7 +236,7 @@ int intel_huc_check_status(struct intel_huc *huc)
> break;
> }
>
> - return huc_is_authenticated(huc);
> + return intel_huc_is_authenticated(huc);
> }
>
> void intel_huc_update_auth_status(struct intel_huc *huc)
> @@ -231,7 +244,7 @@ void intel_huc_update_auth_status(struct intel_huc *huc)
> if (!intel_uc_fw_is_loadable(>fw))
> return;
>
> - if (huc_is_authenticated(huc))
> + if (intel_huc_is_authenticated(huc))
> intel_uc_fw_change_status(>fw,
> INTEL_UC_FIRMWARE_RUNNING);
> }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> index d7e25b6e879e..51f9d96a3ca3 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
> @@ -26,8 +26,10 @@ void intel_huc_init_early(struct intel_huc *huc);
> int intel_huc_init(struct intel_huc *huc);
> void intel_huc_fini(struct intel_huc *huc);
> int intel_huc_auth(struct intel_huc *huc);
> +int intel_huc_wait_for_auth_complete(struct intel_huc *huc);
> int intel_huc_check_status(struct intel_huc *huc);
> void intel_huc_update_auth_status(struct intel_huc *huc);
> +bool intel_huc_is_authenticated(struct intel_huc *huc);
>
> static inline int intel_huc_sanitize(struct intel_huc *huc)
> {
> diff --git
Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_amdkfd.c
Applied. Thanks! On Mon, Sep 5, 2022 at 3:57 AM Jingyu Wang wrote: > > Fix everything checkpatch.pl complained about in amdgpu_amdkfd.c > > Signed-off-by: Jingyu Wang > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > index 091415a4abf0..4f5bd96000ec 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > @@ -1,3 +1,4 @@ > +// SPDX-License-Identifier: MIT > /* > * Copyright 2014 Advanced Micro Devices, Inc. > * > @@ -130,6 +131,7 @@ static void amdgpu_amdkfd_reset_work(struct work_struct > *work) > kfd.reset_work); > > struct amdgpu_reset_context reset_context; > + > memset(_context, 0, sizeof(reset_context)); > > reset_context.method = AMD_RESET_METHOD_NONE; > > base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8 > -- > 2.34.1 >
Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_sync.c file
Applied. Thanks!
Alex
On Mon, Sep 5, 2022 at 2:29 AM Jingyu Wang wrote:
>
> This is a patch to the amdgpu_sync.c file that fixes some warnings found by
> the checkpatch.pl tool
>
> Signed-off-by: Jingyu Wang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> index 504af1b93bfa..090e66a1b284 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: MIT
> /*
> * Copyright 2014 Advanced Micro Devices, Inc.
> * All Rights Reserved.
> @@ -315,6 +316,7 @@ struct dma_fence *amdgpu_sync_get_fence(struct
> amdgpu_sync *sync)
> struct hlist_node *tmp;
> struct dma_fence *f;
> int i;
> +
> hash_for_each_safe(sync->fences, i, tmp, e, node) {
>
> f = e->fence;
> @@ -392,7 +394,7 @@ void amdgpu_sync_free(struct amdgpu_sync *sync)
> {
> struct amdgpu_sync_entry *e;
> struct hlist_node *tmp;
> - unsigned i;
> + unsigned int i;
>
> hash_for_each_safe(sync->fences, i, tmp, e, node) {
> hash_del(>node);
>
> base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
> prerequisite-patch-id: fefd0009b468430bb223fc92e4abe9710518b1ea
> --
> 2.34.1
>
Re: [PATCH] drm/amdgpu: cleanup coding style in amdgpu_acpi.c
Applied. Thanks!
Alex
On Mon, Sep 5, 2022 at 2:29 AM Jingyu Wang wrote:
>
> Fix everything checkpatch.pl complained about in amdgpu_acpi.c
>
> Signed-off-by: Jingyu Wang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 55402d238919..3da27436922c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: MIT
> /*
> * Copyright 2012 Advanced Micro Devices, Inc.
> *
> @@ -849,6 +850,7 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
> if (amdgpu_device_has_dc_support(adev)) {
> #if defined(CONFIG_DRM_AMD_DC)
> struct amdgpu_display_manager *dm = >dm;
> +
> if (dm->backlight_dev[0])
> atif->bd = dm->backlight_dev[0];
> #endif
> @@ -863,6 +865,7 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
> if ((enc->devices &
> (ATOM_DEVICE_LCD_SUPPORT)) &&
> enc->enc_priv) {
> struct amdgpu_encoder_atom_dig *dig =
> enc->enc_priv;
> +
> if (dig->bl_dev) {
> atif->bd = dig->bl_dev;
> break;
> @@ -919,9 +922,9 @@ static bool amdgpu_atif_pci_probe_handle(struct pci_dev
> *pdev)
> return false;
>
> status = acpi_get_handle(dhandle, "ATIF", _handle);
> - if (ACPI_FAILURE(status)) {
> + if (ACPI_FAILURE(status))
> return false;
> - }
> +
> amdgpu_acpi_priv.atif.handle = atif_handle;
> acpi_get_name(amdgpu_acpi_priv.atif.handle, ACPI_FULL_PATHNAME,
> );
> DRM_DEBUG_DRIVER("Found ATIF handle %s\n", acpi_method_name);
> @@ -954,9 +957,9 @@ static bool amdgpu_atcs_pci_probe_handle(struct pci_dev
> *pdev)
> return false;
>
> status = acpi_get_handle(dhandle, "ATCS", _handle);
> - if (ACPI_FAILURE(status)) {
> + if (ACPI_FAILURE(status))
> return false;
> - }
> +
> amdgpu_acpi_priv.atcs.handle = atcs_handle;
> acpi_get_name(amdgpu_acpi_priv.atcs.handle, ACPI_FULL_PATHNAME,
> );
> DRM_DEBUG_DRIVER("Found ATCS handle %s\n", acpi_method_name);
>
> base-commit: e47eb90a0a9ae20b82635b9b99a8d0979b757ad8
> --
> 2.34.1
>
Re: [PATCH linux-next] drm/amdgpu: Remove the unneeded result variable
Applied. Thanks!
Alex
On Fri, Sep 2, 2022 at 4:04 AM wrote:
>
> From: zhang songyi
>
> Return the sdma_v6_0_start() directly instead of storing it in another
> redundant variable.
>
> Reported-by: Zeal Robot
> Signed-off-by: zhang songyi
> ---
> drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> index 2bc1407e885e..2cc2d851b4eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> @@ -1373,12 +1373,9 @@ static int sdma_v6_0_sw_fini(void *handle)
>
> static int sdma_v6_0_hw_init(void *handle)
> {
> - int r;
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - r = sdma_v6_0_start(adev);
> -
> - return r;
> + return sdma_v6_0_start(adev);
> }
>
> static int sdma_v6_0_hw_fini(void *handle)
> --
> 2.25.1
>
>
Re: [PATCH linux-next] drm/radeon: Remove the unneeded result variable
Applied. Thanks!
Alex
On Fri, Sep 2, 2022 at 3:33 AM wrote:
>
> From: ye xingchen
>
> Return the value radeon_drm_ioctl() directly instead of storing it in
> another redundant variable.
>
> Reported-by: Zeal Robot
> Signed-off-by: ye xingchen
> ---
> drivers/gpu/drm/radeon/radeon_drv.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index a28d5ceab628..6cbe1ab81aba 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -512,14 +512,11 @@ long radeon_drm_ioctl(struct file *filp,
> static long radeon_kms_compat_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg)
> {
> unsigned int nr = DRM_IOCTL_NR(cmd);
> - int ret;
>
> if (nr < DRM_COMMAND_BASE)
> return drm_compat_ioctl(filp, cmd, arg);
>
> - ret = radeon_drm_ioctl(filp, cmd, arg);
> -
> - return ret;
> + return radeon_drm_ioctl(filp, cmd, arg);
> }
> #endif
>
> --
> 2.25.1
Re: [PATCH linux-next] drm/radeon/ci_dpm: Remove the unneeded result variable
Applied. Thanks!
On Fri, Sep 2, 2022 at 3:32 AM wrote:
>
> From: ye xingchen
>
> Return the value ci_load_smc_ucode() directly instead of storing it in
> another redundant variable.
>
> Reported-by: Zeal Robot
> Signed-off-by: ye xingchen
> ---
> drivers/gpu/drm/radeon/ci_dpm.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/ci_dpm.c b/drivers/gpu/drm/radeon/ci_dpm.c
> index ac006bed4743..8ef25ab305ae 100644
> --- a/drivers/gpu/drm/radeon/ci_dpm.c
> +++ b/drivers/gpu/drm/radeon/ci_dpm.c
> @@ -2056,7 +2056,7 @@ static void ci_clear_vc(struct radeon_device *rdev)
> static int ci_upload_firmware(struct radeon_device *rdev)
> {
> struct ci_power_info *pi = ci_get_pi(rdev);
> - int i, ret;
> + int i;
>
> for (i = 0; i < rdev->usec_timeout; i++) {
> if (RREG32_SMC(RCU_UC_EVENTS) & BOOT_SEQ_DONE)
> @@ -2067,9 +2067,7 @@ static int ci_upload_firmware(struct radeon_device
> *rdev)
> ci_stop_smc_clock(rdev);
> ci_reset_smc(rdev);
>
> - ret = ci_load_smc_ucode(rdev, pi->sram_end);
> -
> - return ret;
> + return ci_load_smc_ucode(rdev, pi->sram_end);
>
> }
>
> --
> 2.25.1
Re: [PATCH v6 17/57] dyndbg: validate class FOO by checking with module
On Wed, Sep 7, 2022 at 12:19 PM Jason Baron wrote:
>
>
>
> On 9/4/22 17:40, Jim Cromie wrote:
> > Add module-to-class validation:
> >
> > #> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
> >
> > If a query has "class FOO", then ddebug_find_valid_class(), called
> > from ddebug_change(), requires that FOO is known to module X,
> > otherwize the query is skipped entirely for X. This protects each
> > module's class-space, other than the default:31.
> >
> > The authors' choice of FOO is highly selective, giving isolation
> > and/or coordinated sharing of FOOs. For example, only DRM modules
> > should know and respond to DRM_UT_KMS.
> >
> > So this, combined with module's opt-in declaration of known classes,
> > effectively privatizes the .class_id space for each module (or
> > coordinated set of modules).
> >
> > Notes:
> >
> > For all "class FOO" queries, ddebug_find_valid_class() is called, it
> > returns the map matching the query, and sets valid_class via an
> > *outvar).
> >
> > If no "class FOO" is supplied, valid_class = _CLASS_DFLT. This
> > insures that legacy queries do not trample on new class'd callsites,
> > as they get added.
>
>
> Hi Jim,
>
> I'm wondering about the case where we have a callsite which is marked
> as 'class foo', but the query string is done by say module and file, so:
>
> # echo "module bar file foo.c +p" > /proc/dynamic_debug_control
>
> With the proposed code, I think this ends up not enabling anything right?
correct - the only way to enable :pr_debug_cls(CL_FOO, " ...")
is
echo class CL_FOO +p > control
1st, existing dyndbg query uses, whether ad-hoc or scripted,
were not written in anticipation of new / classified subsystems.
2nd, new class users dont want to sit in coach. no damn legroom.
3rd, consider DRM, which already has drm.debug
ie: /sys/module/drm/parameters/debug
and prefers it, at least by inertia.
protecting these new class'd callsites (3-5k of them)
from casual (unintended) manipulations of the kernel-wide
dyndbg state seems prudent, and a usability win.
Not everyone will use module bar, requiring "class foo"
guarantees that changes are intentional.
> Because valid class is set to _DPRINTK_CLASS_DFLT and then:
> 'dp->class_id != valid_class' is true?
>
> This seems confusing to me as a user as this doesn't work like the
> other queriesso maybe we should only do the
> 'dp->class_id != valid_class' check *if* query->class_string is set,
> see below.
>
Could you clarify whether you think this is a logic error
or a frame-of-reference difference as elaborated above ?
ISTM theres a place for a well-worded paragraph in doc
about the class distinction, perhaps a whole for-authors section.
>
>
> >
> > Also add a new column to control-file output, displaying non-default
> > class-name (when found) or the "unknown _id:", if it has not been
> > (correctly) declared with one of the declarator macros.
> >
> > Signed-off-by: Jim Cromie
> > ---
> > lib/dynamic_debug.c | 76 -
> > 1 file changed, 68 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> > index b71efd0b491d..db96ded78c3f 100644
> > --- a/lib/dynamic_debug.c
> > +++ b/lib/dynamic_debug.c
> > @@ -56,6 +56,7 @@ struct ddebug_query {
> > const char *module;
> > const char *function;
> > const char *format;
> > + const char *class_string;
> > unsigned int first_lineno, last_lineno;
> > };
> >
> > @@ -136,15 +137,33 @@ static void vpr_info_dq(const struct ddebug_query
> > *query, const char *msg)
> > fmtlen--;
> > }
> >
> > - v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\"
> > lineno=%u-%u\n",
> > - msg,
> > - query->function ?: "",
> > - query->filename ?: "",
> > - query->module ?: "",
> > - fmtlen, query->format ?: "",
> > - query->first_lineno, query->last_lineno);
> > + v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\"
> > lineno=%u-%u class=%s\n",
> > + msg,
> > + query->function ?: "",
> > + query->filename ?: "",
> > + query->module ?: "",
> > + fmtlen, query->format ?: "",
> > + query->first_lineno, query->last_lineno,
> > query->class_string);
> > }
> >
> > +static struct ddebug_class_map *ddebug_find_valid_class(struct
> > ddebug_table const *dt,
> > + const char
> > *class_string, int *class_id)
> > +{
> > + struct ddebug_class_map *map;
> > + int idx;
> > +
> > + list_for_each_entry(map, >maps, link) {
> > + idx = match_string(map->class_names, map->length,
> > class_string);
> > + if (idx >= 0) {
> > + *class_id = idx + map->base;
> > + return map;
> > + }
> > + }
>
Re: [PATCH -next] drm/amd/display: remove possible condition with no effect (if == else)
Applied. Thanks!
Alex
On Thu, Sep 1, 2022 at 4:34 AM Yang Li wrote:
>
> Conditional statements have no effect to next process.So remove it.
>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2028
> Reported-by: Abaci Robot
> Signed-off-by: Yang Li
> ---
> .../drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git
> a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
> b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
> index e4fd540dec0f..8f1c0e12dd86 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
> @@ -4663,10 +4663,6 @@ void dml32_CalculateMinAndMaxPrefetchMode(
> } else if (AllowForPStateChangeOrStutterInVBlankFinal ==
> dm_prefetch_support_uclk_fclk_and_stutter) {
> *MinPrefetchMode = 0;
> *MaxPrefetchMode = 0;
> - } else if (AllowForPStateChangeOrStutterInVBlankFinal ==
> -
> dm_prefetch_support_uclk_fclk_and_stutter_if_possible) {
> - *MinPrefetchMode = 0;
> - *MaxPrefetchMode = 3;
> } else {
> *MinPrefetchMode = 0;
> *MaxPrefetchMode = 3;
> --
> 2.20.1.7.g153144c
>
Re: [PATCH -next] drm/amd/display: Simplify bool conversion
Applied. Thanks! On Thu, Sep 1, 2022 at 4:11 AM Yang Li wrote: > > The result of relational operation is Boolean, and the question mark > expression is redundant. > > Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2027 > Reported-by: Abaci Robot > Signed-off-by: Yang Li > --- > .../gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git > a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c > b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c > index dc501ee7d01a..e4fd540dec0f 100644 > --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c > +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c > @@ -1873,7 +1873,7 @@ void dml32_CalculateSurfaceSizeInMall( > if (UseMALLForStaticScreen[k] == > dm_use_mall_static_screen_enable) > TotalSurfaceSizeInMALL = TotalSurfaceSizeInMALL + > SurfaceSizeInMALL[k]; > } > - *ExceededMALLSize = (TotalSurfaceSizeInMALL <= MALLAllocatedForDCN * > 1024 * 1024 ? false : true); > + *ExceededMALLSize = (TotalSurfaceSizeInMALL > MALLAllocatedForDCN * > 1024 * 1024); > } // CalculateSurfaceSizeInMall > > void dml32_CalculateVMRowAndSwath( > -- > 2.20.1.7.g153144c >
Re: [Intel-gfx] [PATCH v2] Revert "drm/i915/dg2: extend Wa_1409120013 to DG2"
On Wed, Sep 07, 2022 at 04:25:42PM -0700, Lucas De Marchi wrote: This reverts commit 487970e8bb776c989013bb59d6cbb22e45b9afc6. Updated bspec and workaround database note Wa_1409120013 is not needed for DG2 (or any Xe_LPD) platform. Simply check by display version 12. v2: Simplify condition check to display version (Matt Roper) Cc: Matt Atwood Cc: Clint Taylor Signed-off-by: Lucas De Marchi Reviewed-by: Matt Roper --- drivers/gpu/drm/i915/intel_pm.c | 5 ++--- +Joonas heads up that being a WA I ended up merging this through drm-intel-gt-next. On a hindsight that should probably had gone through drm-intel-next instead since it's a display workaround touching drivers/gpu/drm/i915/intel_pm.c and not the more usual drivers/gpu/drm/i915/gt/intel_workarounds.c Lucas De Marchi
Re: [PATCH -next] drm/amd/display: clean up some inconsistent indentings
Applied. Thanks!
Alex
On Thu, Sep 1, 2022 at 3:57 AM Yang Li wrote:
>
> This if statement is the content of the for statement above it. It
> should be indented.
>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2026
> Reported-by: Abaci Robot
> Signed-off-by: Yang Li
> ---
> drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c
> index 9dd705b985b9..0139e98a0aa1 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c
> @@ -417,8 +417,8 @@ void get_subvp_visual_confirm_color(
> for (i = 0; i < dc->res_pool->pipe_count; i++) {
> struct pipe_ctx *pipe =
> >current_state->res_ctx.pipe_ctx[i];
>
> - if (pipe->stream && pipe->stream->mall_stream_config.paired_stream &&
> - pipe->stream->mall_stream_config.type ==
> SUBVP_MAIN) {
> + if (pipe->stream &&
> pipe->stream->mall_stream_config.paired_stream &&
> + pipe->stream->mall_stream_config.type == SUBVP_MAIN) {
> /* SubVP enable - red */
> color->color_r_cr = color_value;
> enable_subvp = true;
> --
> 2.20.1.7.g153144c
>
Re: [PATCH v3 5/6] drm/sched: Use parent fence instead of finished
On 9/9/2022 11:02 PM, Andrey Grodzovsky wrote:
What exactly is the scenario which this patch fixes in more detail
please ?
GPU reset issue started after adding [PATCH 6/6].
Root cause -> In drm_sched_get_cleanup_job(), We use the finished fence
status bit to check the job status dma_fence_is_signaled(). If a job is
signaled (DMA_FENCE_FLAG_SIGNALED_BIT is set), then we cancel the reset
worker thread.
After applying [patch 6] now we are checking enable signaling in
dma_fence_is_signaled() by checking DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT
bit. but signaling is not enabled for the finished fence. As a result,
dma_fence_is_signaled() always returns false, and
drm_sched_get_cleanup_job() will not cancel the reset worker thread,
resulting in the GPU reset.
To Fix the above issue Christian suggested that we can use
parent(hardware) fence instead of finished fence because signaling
enabled by the calling of dma_fence_add_callback() for parent fence. As
a result, dma_fence_is_signaled() will return the correct fence status
and reset worker thread can be cancelled in drm_sched_get_cleanup_job().
~arvind
Andrey
On 2022-09-09 13:08, Arvind Yadav wrote:
Using the parent fence instead of the finished fence
to get the job status. This change is to avoid GPU
scheduler timeout error which can cause GPU reset.
Signed-off-by: Arvind Yadav
---
changes in v1,v2 - Enable signaling for finished fence in sche_main()
is removed
---
drivers/gpu/drm/scheduler/sched_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index e0ab14e0fb6b..2ac28ad11432 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -829,7 +829,7 @@ drm_sched_get_cleanup_job(struct
drm_gpu_scheduler *sched)
job = list_first_entry_or_null(>pending_list,
struct drm_sched_job, list);
- if (job && dma_fence_is_signaled(>s_fence->finished)) {
+ if (job && dma_fence_is_signaled(job->s_fence->parent)) {
/* remove job from pending_list */
list_del_init(>list);
@@ -841,7 +841,7 @@ drm_sched_get_cleanup_job(struct
drm_gpu_scheduler *sched)
if (next) {
next->s_fence->scheduled.timestamp =
- job->s_fence->finished.timestamp;
+ job->s_fence->parent->timestamp;
/* start TO timer for next job */
drm_sched_start_timeout(sched);
}
[PATCH] drm/amdgpu/display: remove unneeded "default n" options
Remove "default n" options. If the "default" line is removed, it defaults to 'n'. Signed-off-by: Jingyu Wang --- drivers/gpu/drm/amd/display/Kconfig | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig index 413d8c6d592f..6925e0280dbe 100644 --- a/drivers/gpu/drm/amd/display/Kconfig +++ b/drivers/gpu/drm/amd/display/Kconfig @@ -28,7 +28,6 @@ config DRM_AMD_DC_SI bool "AMD DC support for Southern Islands ASICs" depends on DRM_AMDGPU_SI depends on DRM_AMD_DC - default n help Choose this option to enable new AMD DC support for SI asics by default. This includes Tahiti, Pitcairn, Cape Verde, Oland. @@ -43,7 +42,6 @@ config DEBUG_KERNEL_DC config DRM_AMD_SECURE_DISPLAY bool "Enable secure display support" -default n depends on DEBUG_FS depends on DRM_AMD_DC_DCN help base-commit: 5957ac6635a1a12d4aa2661bbf04d3085a73372a -- 2.34.1
Re: [PATCH v2 2/2] pci_ids: Add the various Microsoft PCI device IDs
Please follow the PCI subject line conventions. Discover it with
"git log --oneline include/linux/pci_ids.h".
On Fri, Sep 09, 2022 at 11:50:25AM -0700, Easwar Hariharan wrote:
> From: Easwar Hariharan
>
Needs a commit log, even if it is nothing more than the subject line.
Also read the top of include/linux/pci_ids.h, because it looks like
some of these are only used in one driver and hence do not need to be
in pci_ids.h.
> Signed-off-by: Easwar Hariharan
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 2 +-
> drivers/net/ethernet/microsoft/mana/gdma.h | 3 ---
> drivers/net/ethernet/microsoft/mana/gdma_main.c | 6 +++---
> drivers/video/fbdev/hyperv_fb.c | 4 ++--
> include/linux/pci_ids.h | 4 +++-
> 5 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index f84d397..24c2def 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -51,7 +51,7 @@ static void hyperv_pci_remove(struct pci_dev *pdev)
> static const struct pci_device_id hyperv_pci_tbl[] = {
> {
> .vendor = PCI_VENDOR_ID_MICROSOFT,
> - .device = PCI_DEVICE_ID_HYPERV_VIDEO,
> + .device = PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO,
> },
> { /* end of list */ }
> };
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma.h
> b/drivers/net/ethernet/microsoft/mana/gdma.h
> index 4a6efe6..9d3a9f7 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma.h
> +++ b/drivers/net/ethernet/microsoft/mana/gdma.h
> @@ -476,9 +476,6 @@ struct gdma_eqe {
>
> #define GDMA_SRIOV_REG_CFG_BASE_OFF 0x108
>
> -#define MANA_PF_DEVICE_ID 0x00B9
> -#define MANA_VF_DEVICE_ID 0x00BA
> -
> struct gdma_posted_wqe_info {
> u32 wqe_size_in_bu;
> };
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 00d8198..18cf168 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1333,7 +1333,7 @@ static void mana_gd_cleanup(struct pci_dev *pdev)
>
> static bool mana_is_pf(unsigned short dev_id)
> {
> - return dev_id == MANA_PF_DEVICE_ID;
> + return dev_id == PCI_DEVICE_ID_MICROSOFT_MANA_PF;
> }
>
> static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id
> *ent)
> @@ -1466,8 +1466,8 @@ static void mana_gd_shutdown(struct pci_dev *pdev)
> }
>
> static const struct pci_device_id mana_id_table[] = {
> - { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_PF_DEVICE_ID) },
> - { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_VF_DEVICE_ID) },
> + { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_MICROSOFT_MANA_PF)
> },
> + { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_MICROSOFT_MANA_VF)
> },
> { }
> };
>
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index b58b445..118e244 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -997,7 +997,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct
> fb_info *info)
>
> if (!gen2vm) {
> pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> - PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> + PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO, NULL);
> if (!pdev) {
> pr_err("Unable to find PCI Hyper-V video\n");
> return -ENODEV;
> @@ -1311,7 +1311,7 @@ static int hvfb_resume(struct hv_device *hdev)
> static const struct pci_device_id pci_stub_id_table[] = {
> {
> .vendor = PCI_VENDOR_ID_MICROSOFT,
> - .device = PCI_DEVICE_ID_HYPERV_VIDEO,
> + .device = PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO,
> },
> { /* end of list */ }
> };
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 15b49e6..fe3517f 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2080,7 +2080,9 @@
> #define PCI_DEVICE_ID_VT1724 0x1724
>
> #define PCI_VENDOR_ID_MICROSOFT 0x1414
> -#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
> +#define PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO 0x5353
> +#define PCI_DEVICE_ID_MICROSOFT_MANA_PF 0x00B9
> +#define PCI_DEVICE_ID_MICROSOFT_MANA_VF 0x00BA
>
> #define PCI_VENDOR_ID_OXSEMI 0x1415
> #define PCI_DEVICE_ID_OXSEMI_12PCI8400x8403
> --
> 1.8.3.1
>
Re: [PATCH 2/2] drm/format: Split into more granular test cases
On 8/31/22 18:56, Michał Winiarski wrote: > While we have multiple test cases, most of them check multiple > conditions, calling the function that is tested multiple times with > different arguments (with comments that indicate test case boundary). > This usually means that it can be easily converted into multiple test > cases. > > Passing output: > > = drm_format (18 subtests) = > [PASSED] drm_format_block_width_invalid > [PASSED] drm_format_block_width_one_plane > [PASSED] drm_format_block_width_two_plane > [PASSED] drm_format_block_width_three_plane > [PASSED] drm_format_block_width_tiled > [PASSED] drm_format_block_height_invalid > [PASSED] drm_format_block_height_one_plane > [PASSED] drm_format_block_height_two_plane > [PASSED] drm_format_block_height_three_plane > [PASSED] drm_format_block_height_tiled > [PASSED] drm_format_min_pitch_invalid > [PASSED] drm_format_min_pitch_one_plane_8bpp > [PASSED] drm_format_min_pitch_one_plane_16bpp > [PASSED] drm_format_min_pitch_one_plane_24bpp > [PASSED] drm_format_min_pitch_one_plane_32bpp > [PASSED] drm_format_min_pitch_two_plane > [PASSED] drm_format_min_pitch_three_plane_8bpp > [PASSED] drm_format_min_pitch_tiled > === [PASSED] drm_format > > Testing complete. Ran 18 tests: passed: 18 > > Signed-off-by: Michał Winiarski > --- As there were no more feedback on this, I applied to drm-misc (drm-misc-next). Thanks! Best Regards, - Maíra Canal
Re: [git pull] drm fixes for 6.0-rc5
The pull request you sent on Sat, 10 Sep 2022 01:57:22 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-09-10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/b7e00d6f55015f6995f41c60a5367f1065d37622 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH v6 23/57] drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.
On Wed, Sep 7, 2022 at 12:13 AM Daniel Vetter wrote:
>
> On Sun, Sep 04, 2022 at 03:41:00PM -0600, Jim Cromie wrote:
> > Use DECLARE_DYNDBG_CLASSMAP across DRM:
> >
> > - in .c files, since macro defines/initializes a record
> >
> > - in drivers, $mod_{drv,drm,param}.c
> >ie where param setup is done, since a classmap is param related
> >
> > - in drm/drm_print.c
> >since existing __drm_debug param is defined there,
> >and we ifdef it, and provide an elaborated alternative.
> >
> > - in drm_*_helper modules:
> >dp/drm_dp - 1st item in makefile target
> >drivers/gpu/drm/drm_crtc_helper.c - random pick iirc.
> >
> > Since these modules all use identical CLASSMAP declarations (ie: names
> > and .class_id's) they will all respond together to "class DRM_UT_*"
> > query-commands:
> >
> > :#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
> >
> > NOTES:
> >
> > This changes __drm_debug from int to ulong, so BIT() is usable on it.
> >
> > DRM's enum drm_debug_category values need to sync with the index of
> > their respective class-names here. Then .class_id == category, and
> > dyndbg's class FOO mechanisms will enable drm_dbg(DRM_UT_KMS, ...).
> >
> > Though DRM needs consistent categories across all modules, thats not
> > generally needed; modules X and Y could define FOO differently (ie a
> > different NAME => class_id mapping), changes are made according to
> > each module's private class-map.
> >
> > No callsites are actually selected by this patch, since none are
> > class'd yet.
> >
> > Signed-off-by: Jim Cromie
>
> So maybe I should just try, but what happens if a drm module doesn't have
> these classbits declared? You simply have to use the raw number instead?
without the classnames declared via macro,
dyndbg has no names by which to validate the query.
raw class numbers are not usable into >control.
This is what privatizes the module's class-id space.
If the macro is missing, the drm_dbg()s ( after conversion to reside
atop dyndbg)
will do this in `cat control`
seq_printf(m, " class unknown, _id:%d", dp->class_id);
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 +
> > drivers/gpu/drm/display/drm_dp_helper.c | 13
> > drivers/gpu/drm/drm_crtc_helper.c | 13
> > drivers/gpu/drm/drm_print.c | 27 +++--
> > drivers/gpu/drm/i915/i915_params.c | 12 +++
> > drivers/gpu/drm/nouveau/nouveau_drm.c | 13
> > include/drm/drm_print.h | 3 ++-
> > 7 files changed, 92 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index de7144b06e93..97e184f44a52 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -38,6 +38,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> >
> > #include "amdgpu.h"
> > #include "amdgpu_irq.h"
> > @@ -185,6 +187,18 @@ int amdgpu_vcnfw_log;
> >
> > static void amdgpu_drv_delayed_reset_work_handler(struct work_struct
> > *work);
> >
> > +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
>
> Iirc we've talked about maybe some kbuild trickery so that any module
> under drivers/gpu/drm gets these by default. I don't think we need to have
> this for the first cut, but a macro to avoid the copypaste mistakes would
> be really good here.
It *may be* that theres a perfect place to declare it once, for everyone.
For me thats exploratory, error prone.
Proving that the sub-optimal worked seemed a good place to stop.
that said, theres a macro in test-dynamic-debug that is a candidate
for wider availability - it needs a better name
#define DD_SYS_WRAP(_model, _flags) \
static unsigned long bits_##_model; \
static struct ddebug_class_param _flags##_model = { \
.bits = _##_model, \
.flags = #_flags, \
.map = _##_model, \
}; \
module_param_cb(_flags##_##_model, _ops_dyndbg_classes,
&_flags##_model, 0600)
[PATCH v2 2/2] pci_ids: Add the various Microsoft PCI device IDs
From: Easwar Hariharan
Signed-off-by: Easwar Hariharan
---
drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 2 +-
drivers/net/ethernet/microsoft/mana/gdma.h | 3 ---
drivers/net/ethernet/microsoft/mana/gdma_main.c | 6 +++---
drivers/video/fbdev/hyperv_fb.c | 4 ++--
include/linux/pci_ids.h | 4 +++-
5 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index f84d397..24c2def 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -51,7 +51,7 @@ static void hyperv_pci_remove(struct pci_dev *pdev)
static const struct pci_device_id hyperv_pci_tbl[] = {
{
.vendor = PCI_VENDOR_ID_MICROSOFT,
- .device = PCI_DEVICE_ID_HYPERV_VIDEO,
+ .device = PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO,
},
{ /* end of list */ }
};
diff --git a/drivers/net/ethernet/microsoft/mana/gdma.h
b/drivers/net/ethernet/microsoft/mana/gdma.h
index 4a6efe6..9d3a9f7 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma.h
+++ b/drivers/net/ethernet/microsoft/mana/gdma.h
@@ -476,9 +476,6 @@ struct gdma_eqe {
#define GDMA_SRIOV_REG_CFG_BASE_OFF0x108
-#define MANA_PF_DEVICE_ID 0x00B9
-#define MANA_VF_DEVICE_ID 0x00BA
-
struct gdma_posted_wqe_info {
u32 wqe_size_in_bu;
};
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 00d8198..18cf168 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1333,7 +1333,7 @@ static void mana_gd_cleanup(struct pci_dev *pdev)
static bool mana_is_pf(unsigned short dev_id)
{
- return dev_id == MANA_PF_DEVICE_ID;
+ return dev_id == PCI_DEVICE_ID_MICROSOFT_MANA_PF;
}
static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
@@ -1466,8 +1466,8 @@ static void mana_gd_shutdown(struct pci_dev *pdev)
}
static const struct pci_device_id mana_id_table[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_PF_DEVICE_ID) },
- { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_VF_DEVICE_ID) },
+ { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_MICROSOFT_MANA_PF)
},
+ { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_MICROSOFT_MANA_VF)
},
{ }
};
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index b58b445..118e244 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -997,7 +997,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct
fb_info *info)
if (!gen2vm) {
pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
- PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
+ PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO, NULL);
if (!pdev) {
pr_err("Unable to find PCI Hyper-V video\n");
return -ENODEV;
@@ -1311,7 +1311,7 @@ static int hvfb_resume(struct hv_device *hdev)
static const struct pci_device_id pci_stub_id_table[] = {
{
.vendor = PCI_VENDOR_ID_MICROSOFT,
- .device = PCI_DEVICE_ID_HYPERV_VIDEO,
+ .device = PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO,
},
{ /* end of list */ }
};
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 15b49e6..fe3517f 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2080,7 +2080,9 @@
#define PCI_DEVICE_ID_VT1724 0x1724
#define PCI_VENDOR_ID_MICROSOFT0x1414
-#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
+#define PCI_DEVICE_ID_MICROSOFT_HYPERV_VIDEO 0x5353
+#define PCI_DEVICE_ID_MICROSOFT_MANA_PF0x00B9
+#define PCI_DEVICE_ID_MICROSOFT_MANA_VF0x00BA
#define PCI_VENDOR_ID_OXSEMI 0x1415
#define PCI_DEVICE_ID_OXSEMI_12PCI840 0x8403
--
1.8.3.1
Re: [PATCH 0/4] drm/plane: Remove drm_plane_init(), plus other cleanups
For the nouveau bits on 1, 2 and 4: Reviewed-by: Lyude Paul On Fri, 2022-09-09 at 12:59 +0200, Thomas Zimmermann wrote: > This patchset does cleanups to the plane code, most noteably it removes > drm_plane_init(). The function is a small wrapper, which can easily be > inlined into the few callers. Patch #1 fixes this. > > The other clean-up patches #2 to #4 affect plane creation. Modesetting > helpers and nouveau share some plane-allocation code that can be shared as > helper function. While the function is already outdated, it's now at least > well documented. As suggested by Daniel, patch #3 adds a warning to > non-atomic plane helpers when they are being called from atomic drivers. > Patch #4 adds an initializer macro for non-atomic plane functions. It > should not be used in new drivers, but at least documents the current > practice. > > Tested with nouveau on Nvidia G72 hardware. > > A possible next step would be the inlining of drm_crtc_init() and the > removal of drm_plane.format_default. > > Thomas Zimmermann (4): > drm/plane: Remove drm_plane_init() > drm/plane: Allocate planes with drm_universal_plane_alloc() > drm/plane-helper: Warn if atomic drivers call non-atomic helpers > drm/plane-helper: Provide DRM_PLANE_NON_ATOMIC_FUNCS initializer macro > > drivers/gpu/drm/drm_modeset_helper.c | 68 + > drivers/gpu/drm/drm_plane.c| 70 -- > drivers/gpu/drm/drm_plane_helper.c | 10 > drivers/gpu/drm/nouveau/dispnv04/crtc.c| 45 +- > drivers/gpu/drm/nouveau/dispnv04/overlay.c | 13 ++-- > drivers/gpu/drm/shmobile/shmob_drm_plane.c | 7 ++- > drivers/gpu/drm/tilcdc/tilcdc_plane.c | 9 ++- > include/drm/drm_plane.h| 52 +--- > include/drm/drm_plane_helper.h | 12 > 9 files changed, 162 insertions(+), 124 deletions(-) > > > base-commit: f2c3a05d33693ad51996fa7d12d3b2d4b0f372eb > prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d > prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 > prerequisite-patch-id: 3f204510fcbf9530d6540bd8e6128cce598988b6 -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH 1/4] drm/plane: Remove drm_plane_init()
Reviewed-by: Lyude Paul
for common and nouveau bits
On Fri, 2022-09-09 at 12:59 +0200, Thomas Zimmermann wrote:
> Open-code drm_plane_init() and remove the function from DRM. The
> implementation of drm_plane_init() is a simple wrapper around a call
> to drm_universal_plane_init(), so drivers can just use that instead.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_modeset_helper.c | 3 +-
> drivers/gpu/drm/drm_plane.c| 32 --
> drivers/gpu/drm/nouveau/dispnv04/overlay.c | 13 +
> drivers/gpu/drm/shmobile/shmob_drm_plane.c | 7 +++--
> drivers/gpu/drm/tilcdc/tilcdc_plane.c | 9 +++---
> include/drm/drm_plane.h| 8 +-
> 6 files changed, 17 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c
> b/drivers/gpu/drm/drm_modeset_helper.c
> index bd609a978848..611dd01fb604 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -100,8 +100,7 @@ EXPORT_SYMBOL(drm_helper_mode_fill_fb_struct);
> * This is the minimal list of formats that seem to be safe for modeset use
> * with all current DRM drivers. Most hardware can actually support more
> * formats than this and drivers may specify a more accurate list when
> - * creating the primary plane. However drivers that still call
> - * drm_plane_init() will use this minimal format list as the default.
> + * creating the primary plane.
> */
> static const uint32_t safe_modeset_formats[] = {
> DRM_FORMAT_XRGB,
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 726f2f163c26..0f14b4d3bb10 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -482,38 +482,6 @@ void drm_plane_unregister_all(struct drm_device *dev)
> }
> }
>
> -/**
> - * drm_plane_init - Initialize a legacy plane
> - * @dev: DRM device
> - * @plane: plane object to init
> - * @possible_crtcs: bitmask of possible CRTCs
> - * @funcs: callbacks for the new plane
> - * @formats: array of supported formats (DRM_FORMAT\_\*)
> - * @format_count: number of elements in @formats
> - * @is_primary: plane type (primary vs overlay)
> - *
> - * Legacy API to initialize a DRM plane.
> - *
> - * New drivers should call drm_universal_plane_init() instead.
> - *
> - * Returns:
> - * Zero on success, error code on failure.
> - */
> -int drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
> -uint32_t possible_crtcs,
> -const struct drm_plane_funcs *funcs,
> -const uint32_t *formats, unsigned int format_count,
> -bool is_primary)
> -{
> - enum drm_plane_type type;
> -
> - type = is_primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
> - return drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
> - formats, format_count,
> - NULL, type, NULL);
> -}
> -EXPORT_SYMBOL(drm_plane_init);
> -
> /**
> * drm_plane_cleanup - Clean up the core plane usage
> * @plane: plane to cleanup
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> index 37e63e98cd08..33f29736024a 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
> @@ -296,9 +296,10 @@ nv10_overlay_init(struct drm_device *device)
> break;
> }
>
> - ret = drm_plane_init(device, >base, 3 /* both crtc's */,
> - _plane_funcs,
> - formats, num_formats, false);
> + ret = drm_universal_plane_init(device, >base, 3 /* both crtc's
> */,
> +_plane_funcs,
> +formats, num_formats, NULL,
> +DRM_PLANE_TYPE_OVERLAY, NULL);
> if (ret)
> goto err;
>
> @@ -475,9 +476,9 @@ nv04_overlay_init(struct drm_device *device)
> if (!plane)
> return;
>
> - ret = drm_plane_init(device, >base, 1 /* single crtc */,
> - _plane_funcs,
> - formats, 2, false);
> + ret = drm_universal_plane_init(device, >base, 1 /* single crtc
> */,
> +_plane_funcs, formats, 2, NULL,
> +DRM_PLANE_TYPE_OVERLAY, NULL);
> if (ret)
> goto err;
>
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> index 54228424793a..6c5f0cbe7d95 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
> @@ -252,9 +252,10 @@ int shmob_drm_plane_create(struct shmob_drm_device
> *sdev, unsigned int index)
> splane->index = index;
> splane->alpha = 255;
>
> - ret = drm_plane_init(sdev->ddev, >plane, 1,
Re: [Intel-gfx] Developing a new backlight driver for specific OLED screen
On Fri, 2022-09-09 at 15:49 +, Aurélien wrote: > Hi, > > > + dri-devel mailing list that looks more appropriated. > > + Hans and Lyude who were recently working to standardize some of the > > backlight stuff. > > Thank you for these contacts. I'll try there if I need. > > > I don't believe you want to use the i915 API, but move the common functions > > to the drm subsystem itself and then reuse as a drm device. > > If there is enough generic code I'll do everything with the DRM API. > Unfortunately I can't spend too much time in order to generalize the i915 > common functions. > > > Aurélien, are you aware of drivers/gpu/drm/display/drm_dp_helper.c and > > all the functions around struct dp_aux_backlight and struct > > drm_edp_backlight_info? > > Not yet. Since I'm not familiar with GPU/display drivers I didn't know what > could be a good starting point. > Indeed I already checked the intel_dp_aux_backlight.c code. That's why I told > about using the "i915 API code" at first. But since this display is > independent from the GPU i didn't want to link both code. > Then that's a really good point if there is already an independant API. I'll > have a look this evening. > > > Does the display use some proprietary, non-VESA DPCD registers? There's > > already some of that in i915 for Intel proprietary interfaces. > > For sure. It's an OLED display. Thus there is no backlight. It uses specific > registers to control the brightness of the screen. > Unfortunately I guess the mechanism is not shared with many OLED displays... > > Thank you for your help. Seems like we've got a pretty good argument for renaming this from backlight to something else at some point. FWIW, the word backlight is sometimes used around these parts simply to describe controlling the brightness of an OLED screen. > > Aurélien > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH v4 06/21] drm/i915: Prepare to dynamic dma-buf locking specification
On 9/2/22 19:26, Ruhl, Michael J wrote: >> 02.09.2022 13:31, Dmitry Osipenko пишет: >>> 01.09.2022 17:02, Ruhl, Michael J пишет: >>> ... > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c > @@ -331,7 +331,19 @@ static void __i915_gem_free_objects(struct > drm_i915_private *i915, > continue; > } > > + /* > + * dma_buf_unmap_attachment() requires reservation to be > + * locked. The imported GEM shouldn't share reservation lock, > + * so it's safe to take the lock. > + */ > + if (obj->base.import_attach) > + i915_gem_object_lock(obj, NULL); There is a lot of stuff going here. Taking the lock may be premature... > __i915_gem_object_pages_fini(obj); The i915_gem_dmabuf.c:i915_gem_object_put_pages_dmabuf is where unmap_attachment is actually called, would it make more sense to make do the locking there? >>> >>> The __i915_gem_object_put_pages() is invoked with a held reservation >>> lock, while freeing object is a special time when we know that GEM is >>> unused. >>> >>> The __i915_gem_free_objects() was taking the lock two weeks ago until >>> the change made Chris Wilson [1] reached linux-next. >>> >>> [1] >>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux- >> next.git/commit/?id=2826d447fbd60e6a05e53d5f918bceb8c04e315c >>> >>> I don't think we can take the lock within >>> i915_gem_object_put_pages_dmabuf(), it may/should deadlock other code >> paths. >> >> On the other hand, we can check whether the GEM's refcount number is >> zero in i915_gem_object_put_pages_dmabuf() and then take the lock if >> it's zero. >> >> Also, seems it should be possible just to bail out from >> i915_gem_object_put_pages_dmabuf() if refcount=0. The further >> drm_prime_gem_destroy() will take care of unmapping. Perhaps this could >> be the best option, I'll give it a test. > > i915_gem_object_put_pages() is uses the SG, and the usage for > drm_prim_gem_destroy() > > from __i915_gem_free_objects() doesn't use the SG because it has been "freed" > already, I am not sure if that would work... > > Hmm.. with that in mind, maybe moving the base.import_attach check to > __i915_gem_object_put_pages with your attach check? I see you meant __i915_gem_object_pages_fini() here. > atomic_set(>mm.pages_pin_count, 0); > if (obj->base.import) > i915_gem_object_lock(obj, NULL); > > __i915_gem_object_put_pages(obj); > > if (obj->base.import) > i915_gem_object_unlock(obj, NULL); > GEM_BUG_ON(i915_gem_object_has_pages(obj)); > > Pretty much one step up from the dmabuf interface, but we are guaranteed to > not have any pinned pages? Importer shouldn't hold pages outside of dma-buf API, otherwise it should be a bug. > The other caller of __i915_gem_object_pages_fini is the i915_ttm move_notify > which should not conflict (export side, not import side). > > Since it appears that not locking during the clean up is desirable, trying to > make sure take the lock > is taken at the last moment might be the right path? Reducing the scope of locking usually is preferred more. Yours suggestion works okay, I couldn't spot any problems at least for a non-TTM code paths. It's indeed a bit not nice that __i915_gem_object_pages_fini() is used by TTM, but should be safe for imported objects. Will be great if anyone from i915 maintainers could ack this variant. -- Best regards, Dmitry
Re: [PATCH v3 5/6] drm/sched: Use parent fence instead of finished
What exactly is the scenario which this patch fixes in more detail please ?
Andrey
On 2022-09-09 13:08, Arvind Yadav wrote:
Using the parent fence instead of the finished fence
to get the job status. This change is to avoid GPU
scheduler timeout error which can cause GPU reset.
Signed-off-by: Arvind Yadav
---
changes in v1,v2 - Enable signaling for finished fence in sche_main()
is removed
---
drivers/gpu/drm/scheduler/sched_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index e0ab14e0fb6b..2ac28ad11432 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -829,7 +829,7 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched)
job = list_first_entry_or_null(>pending_list,
struct drm_sched_job, list);
- if (job && dma_fence_is_signaled(>s_fence->finished)) {
+ if (job && dma_fence_is_signaled(job->s_fence->parent)) {
/* remove job from pending_list */
list_del_init(>list);
@@ -841,7 +841,7 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched)
if (next) {
next->s_fence->scheduled.timestamp =
- job->s_fence->finished.timestamp;
+ job->s_fence->parent->timestamp;
/* start TO timer for next job */
drm_sched_start_timeout(sched);
}
Re: [PATCH v8] drm: Add initial ci/ subdirectory
Hi, On Fri, 9 Sept 2022 at 15:15, Tomeu Vizoso wrote: > Also include a configuration file that points to the out-of-tree CI > scripts. > I think this para is outdated given ... v8: > - Move all files specific to testing the kernel into the kernel tree > (thus I have dropped the r-bs I had collected so far) > - Uprev Gitlab CI infrastructure scripts to the latest from Mesa But equally - and sorry for not jumping on the IRC (?) discussion as I was in the middle of other stuff when it came up - I'm don't think this is the right plan. Mesa having all its CI in-tree makes sense, because merges happen rapidly to a single canonical tree. If the scripts need to be changed for whatever reason, we can merge something in under an hour and everyone immediately gets it. DRM is quite different though, given the forest of trees we have and the long merge paths between them. I worry that merging the CI scripts in-tree - especially for our initial attempt at it, when we're likely to need to make quite a lot of changes before it settles down to become a stable system that works for everyone - is shooting ourselves in the foot by limiting our flexibility. Given that it's currently very dependent on fd.o infrastructure (published ci-templates, the registry, the specific-tag runners for Collabora/CrOS DUTs, etc), there isn't much of a portability gain in bringing the scripts into the tree either. It's a good goal, but not where we are today. Cheers, Daniel
[PATCH v2] drm/msm/dp: add atomic_check to bridge ops
DRM commit_tails() will disable downstream crtc/encoder/bridge if
both disable crtc is required and crtc->active is set before pushing
a new frame downstream.
There is a rare case that user space display manager issue an extra
screen update immediately followed by close DRM device while down
stream display interface is disabled. This extra screen update will
timeout due to the downstream interface is disabled but will cause
crtc->active be set. Hence the followed commit_tails() called by
drm_release() will pass the disable downstream crtc/encoder/bridge
conditions checking even downstream interface is disabled.
This cause the crash to happen at dp_bridge_disable() due to it trying
to access the main link register to push the idle pattern out while main
link clocks is disabled.
This patch adds atomic_check to prevent the extra frame will not
be pushed down if display interface is down so that crtc->active
will not be set neither. This will fail the conditions checking
of disabling down stream crtc/encoder/bridge which prevent
drm_release() from calling dp_bridge_disable() so that crash
at dp_bridge_disable() prevented.
There is no protection in the DRM framework to check if the display
pipeline has been already disabled before trying again. The only
check is the crtc_state->active but this is controlled by usermode
using UAPI. Hence if the usermode sets this and then crashes, the
driver needs to protect against double disable"
SError Interrupt on CPU7, code 0xbe000411 -- SError
CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
Hardware name: Google Lazor (rev3 - 8) (DT)
pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __cmpxchg_case_acq_32+0x14/0x2c
lr : do_raw_spin_lock+0xa4/0xdc
sp : ffc01092b6a0
x29: ffc01092b6a0 x28: 0028 x27: 0038
x26: 0004 x25: ffd2973dce48 x24:
x23: x22: x21: ffd2978d0008
x20: ffd2978d0008 x19: ff80ff759fc0 x18:
x17: 004800a501260460 x16: 0441043b04600438 x15: 0438089807d0
x14: 07b0089807800780 x13: x12:
x11: 0438 x10: 07d0 x9 : ffd2973e09e4
x8 : ff8092d53300 x7 : ff808902e8b8 x6 : 0001
x5 : ff808902e880 x4 : x3 : ff80ff759fc0
x2 : 0001 x1 : x0 : ff80ff759fc0
Kernel panic - not syncing: Asynchronous SError Interrupt
CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
Hardware name: Google Lazor (rev3 - 8) (DT)
Call trace:
dump_backtrace.part.0+0xbc/0xe4
show_stack+0x24/0x70
dump_stack_lvl+0x68/0x84
dump_stack+0x18/0x34
panic+0x14c/0x32c
nmi_panic+0x58/0x7c
arm64_serror_panic+0x78/0x84
do_serror+0x40/0x64
el1h_64_error_handler+0x30/0x48
el1h_64_error+0x68/0x6c
__cmpxchg_case_acq_32+0x14/0x2c
_raw_spin_lock_irqsave+0x38/0x4c
lock_timer_base+0x40/0x78
__mod_timer+0xf4/0x25c
schedule_timeout+0xd4/0xfc
__wait_for_common+0xac/0x140
wait_for_completion_timeout+0x2c/0x54
dp_ctrl_push_idle+0x40/0x88
dp_bridge_disable+0x24/0x30
drm_atomic_bridge_chain_disable+0x90/0xbc
drm_atomic_helper_commit_modeset_disables+0x198/0x444
msm_atomic_commit_tail+0x1d0/0x374
commit_tail+0x80/0x108
drm_atomic_helper_commit+0x118/0x11c
drm_atomic_commit+0xb4/0xe0
drm_client_modeset_commit_atomic+0x184/0x224
drm_client_modeset_commit_locked+0x58/0x160
drm_client_modeset_commit+0x3c/0x64
__drm_fb_helper_restore_fbdev_mode_unlocked+0x98/0xac
drm_fb_helper_set_par+0x74/0x80
drm_fb_helper_hotplug_event+0xdc/0xe0
__drm_fb_helper_restore_fbdev_mode_unlocked+0x7c/0xac
drm_fb_helper_restore_fbdev_mode_unlocked+0x20/0x2c
drm_fb_helper_lastclose+0x20/0x2c
drm_lastclose+0x44/0x6c
drm_release+0x88/0xd4
__fput+0x104/0x220
fput+0x1c/0x28
task_work_run+0x8c/0x100
do_exit+0x450/0x8d0
do_group_exit+0x40/0xac
__wake_up_parent+0x0/0x38
invoke_syscall+0x84/0x11c
el0_svc_common.constprop.0+0xb8/0xe4
do_el0_svc+0x8c/0xb8
el0_svc+0x2c/0x54
el0t_64_sync_handler+0x120/0x1c0
el0t_64_sync+0x190/0x194
SMP: stopping secondary CPUs
Kernel Offset: 0x128e80 from 0xffc00800
PHYS_OFFSET: 0x8000
CPU features: 0x800,00c2a015,19801c82
Memory Limit: none
Changes in v2:
-- add more commit text
Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable
and disable")
Reported-by: Leonard Lausen
Suggested-by: Rob Clark
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/17
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_drm.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 6df25f7..c682588 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -31,6 +31,25 @@ static enum drm_connector_status dp_bridge_detect(struct
drm_bridge *bridge)
connector_status_disconnected;
Re: [Intel-gfx] [PATCH v4 11/11] drm/i915/mtl: Do not update GV point, mask value
On 01.09.2022 23:03, Radhakrishna Sripada wrote:
> Display 14 and future platforms do not directly communicate to Pcode
> via mailbox the SAGV bandwidth information. PM Demand registers are
> used to communicate display power requirements to the PUnit which would
> include GV point and mask value.
>
> Skip programming GV point and mask values through legacy pcode mailbox
> interface.
I agree to Matt's suggestion in v2 of this patch series, to move this
patch to the future series where we would introduce the new pm_demand
interface. It would make more sense there.
>
> Bspec: 64636
> Cc: Matt Roper
> Original Author: Caz Yokoyama
> Signed-off-by: Radhakrishna Sripada
> ---
> drivers/gpu/drm/i915/intel_pm.c | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b19a1ecb010e..69efd613bbde 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3923,6 +3923,14 @@ void intel_sagv_pre_plane_update(struct
> intel_atomic_state *state)
> {
> struct drm_i915_private *i915 = to_i915(state->base.dev);
>
> + /*
> + * No need to update mask value/restrict because
> + * "Pcode only wants to use GV bandwidth value, not the mask value."
> + * for DISPLAY_VER() >= 14.
> + */
> + if (DISPLAY_VER(i915) >= 14)
> + return;
> +
My suggestion would be to remove the DISPLAY version check here and do
it at the place where this function is invoked from. So for versions
<14, intel_sagv_pre_plane_update can be called and for higher we need to
implement the new pm_demand interface.
> /*
>* Just return if we can't control SAGV or don't have it.
>* This is different from situation when we have SAGV but just can't
> @@ -3943,6 +3951,16 @@ void intel_sagv_post_plane_update(struct
> intel_atomic_state *state)
> {
> struct drm_i915_private *i915 = to_i915(state->base.dev);
>
> + /*
> + * No need to update mask value/restrict because
> + * "Pcode only wants to use GV bandwidth value, not the mask value."
> + * for DISPLAY_VER() >= 14.
> + *
> + * GV bandwidth will be set by intel_pmdemand_post_plane_update()
> + */
> + if (DISPLAY_VER(i915) >= 14)
> + return;
ditto
> +
> /*
>* Just return if we can't control SAGV or don't have it.
>* This is different from situation when we have SAGV but just can't
Regards,
Bala
> --
> 2.34.1
>
[PATCH v3 6/6] dma-buf: Check status of enable-signaling bit on debug
Fence signaling must be enabled to make sure that
the dma_fence_is_signaled() function ever returns true.
Since drivers and implementations sometimes mess this up,
this ensures correct behaviour when DEBUG_WW_MUTEX_SLOWPATH
is used during debugging.
This should make any implementation bugs resulting in not
signaled fences much more obvious.
Signed-off-by: Arvind Yadav
---
Changes in v1,v2 :
1- Addressing Christian's comment to replace
CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS.
2- As per Christian's comment moving this patch at last so
The version of this patch is also changed and previously
it was [PATCH 1/4]
---
include/linux/dma-fence.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 775cdc0b4f24..ba1ddc14c5d4 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -428,6 +428,11 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
static inline bool
dma_fence_is_signaled(struct dma_fence *fence)
{
+#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
+ if (!test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags))
+ return false;
+#endif
+
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags))
return true;
--
2.25.1
[PATCH v3 5/6] drm/sched: Use parent fence instead of finished
Using the parent fence instead of the finished fence
to get the job status. This change is to avoid GPU
scheduler timeout error which can cause GPU reset.
Signed-off-by: Arvind Yadav
---
changes in v1,v2 - Enable signaling for finished fence in sche_main()
is removed
---
drivers/gpu/drm/scheduler/sched_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index e0ab14e0fb6b..2ac28ad11432 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -829,7 +829,7 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched)
job = list_first_entry_or_null(>pending_list,
struct drm_sched_job, list);
- if (job && dma_fence_is_signaled(>s_fence->finished)) {
+ if (job && dma_fence_is_signaled(job->s_fence->parent)) {
/* remove job from pending_list */
list_del_init(>list);
@@ -841,7 +841,7 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched)
if (next) {
next->s_fence->scheduled.timestamp =
- job->s_fence->finished.timestamp;
+ job->s_fence->parent->timestamp;
/* start TO timer for next job */
drm_sched_start_timeout(sched);
}
--
2.25.1
[PATCH v3 4/6] drm/amdgpu: Enable signaling on fence.
Here's enabling software signaling on fence because amdgpu_ctx_add_fence() is checking the status of fence and emits warning. Signed-off-by: Arvind Yadav --- Changes in v1, v2: This new patch was not part of previous series. --- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c index afe22f83d4a6..21221d705588 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c @@ -730,6 +730,8 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, dma_fence_get(fence); + dma_fence_enable_sw_signaling(fence); + spin_lock(>ring_lock); centity->fences[idx] = fence; centity->sequence++; -- 2.25.1
[PATCH v3 3/6] dma-buf: Enable signaling on fence for selftests
Here's enabling software signaling on fence for selftest.
Signed-off-by: Arvind Yadav
---
Changes in v1 :
1- Addressing Christian's comment to remove unnecessary callback.
2- Replacing CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS.
3- The version of this patch is also changed and previously
it was [PATCH 4/4]
Changes in v2 :
1- #ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH removed
---
drivers/dma-buf/st-dma-fence-chain.c | 4
drivers/dma-buf/st-dma-fence-unwrap.c | 22 ++
drivers/dma-buf/st-dma-fence.c| 16
drivers/dma-buf/st-dma-resv.c | 10 ++
4 files changed, 52 insertions(+)
diff --git a/drivers/dma-buf/st-dma-fence-chain.c
b/drivers/dma-buf/st-dma-fence-chain.c
index 8ce1ea59d31b..0a9b099d0518 100644
--- a/drivers/dma-buf/st-dma-fence-chain.c
+++ b/drivers/dma-buf/st-dma-fence-chain.c
@@ -87,6 +87,8 @@ static int sanitycheck(void *arg)
if (!chain)
err = -ENOMEM;
+ dma_fence_enable_sw_signaling(chain);
+
dma_fence_signal(f);
dma_fence_put(f);
@@ -143,6 +145,8 @@ static int fence_chains_init(struct fence_chains *fc,
unsigned int count,
}
fc->tail = fc->chains[i];
+
+ dma_fence_enable_sw_signaling(fc->chains[i]);
}
fc->chain_length = i;
diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c
b/drivers/dma-buf/st-dma-fence-unwrap.c
index 4105d5ea8dde..f0cee984b6c7 100644
--- a/drivers/dma-buf/st-dma-fence-unwrap.c
+++ b/drivers/dma-buf/st-dma-fence-unwrap.c
@@ -102,6 +102,8 @@ static int sanitycheck(void *arg)
if (!f)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f);
+
array = mock_array(1, f);
if (!array)
return -ENOMEM;
@@ -124,12 +126,16 @@ static int unwrap_array(void *arg)
if (!f1)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f1);
+
f2 = mock_fence();
if (!f2) {
dma_fence_put(f1);
return -ENOMEM;
}
+ dma_fence_enable_sw_signaling(f2);
+
array = mock_array(2, f1, f2);
if (!array)
return -ENOMEM;
@@ -164,12 +170,16 @@ static int unwrap_chain(void *arg)
if (!f1)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f1);
+
f2 = mock_fence();
if (!f2) {
dma_fence_put(f1);
return -ENOMEM;
}
+ dma_fence_enable_sw_signaling(f2);
+
chain = mock_chain(f1, f2);
if (!chain)
return -ENOMEM;
@@ -204,12 +214,16 @@ static int unwrap_chain_array(void *arg)
if (!f1)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f1);
+
f2 = mock_fence();
if (!f2) {
dma_fence_put(f1);
return -ENOMEM;
}
+ dma_fence_enable_sw_signaling(f2);
+
array = mock_array(2, f1, f2);
if (!array)
return -ENOMEM;
@@ -248,12 +262,16 @@ static int unwrap_merge(void *arg)
if (!f1)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f1);
+
f2 = mock_fence();
if (!f2) {
err = -ENOMEM;
goto error_put_f1;
}
+ dma_fence_enable_sw_signaling(f2);
+
f3 = dma_fence_unwrap_merge(f1, f2);
if (!f3) {
err = -ENOMEM;
@@ -296,10 +314,14 @@ static int unwrap_merge_complex(void *arg)
if (!f1)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f1);
+
f2 = mock_fence();
if (!f2)
goto error_put_f1;
+ dma_fence_enable_sw_signaling(f2);
+
f3 = dma_fence_unwrap_merge(f1, f2);
if (!f3)
goto error_put_f2;
diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c
index c8a12d7ad71a..fb6e0a6ae2c9 100644
--- a/drivers/dma-buf/st-dma-fence.c
+++ b/drivers/dma-buf/st-dma-fence.c
@@ -102,6 +102,8 @@ static int sanitycheck(void *arg)
if (!f)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f);
+
dma_fence_signal(f);
dma_fence_put(f);
@@ -117,6 +119,8 @@ static int test_signaling(void *arg)
if (!f)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f);
+
if (dma_fence_is_signaled(f)) {
pr_err("Fence unexpectedly signaled on creation\n");
goto err_free;
@@ -190,6 +194,8 @@ static int test_late_add_callback(void *arg)
if (!f)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f);
+
dma_fence_signal(f);
if (!dma_fence_add_callback(f, , simple_callback)) {
@@ -282,6 +288,8 @@ static int test_status(void *arg)
if (!f)
return -ENOMEM;
+ dma_fence_enable_sw_signaling(f);
+
if (dma_fence_get_status(f)) {
[PATCH v3 2/6] dma-buf: set signaling bit for the stub fence
Here's setting software signaling bit for the stub fence which is always signaled. If this fence signaling bit is not set then the AMD GPU scheduler will cause a GPU reset due to a GPU scheduler cleanup activity timeout. Signed-off-by: Arvind Yadav --- Changes in v1 : 1- Addressing Christian's comment to remove unnecessary callback. 2- Replacing CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS. 3- The version of this patch is also changed and previously it was [PATCH 3/4] Changes in v2 : 1 - perviously using __dma_fence_enable_signaling() for enable signaling. 2 - #ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH removed --- drivers/dma-buf/dma-fence.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 64c99739ad23..bead1a6e9f59 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -136,6 +136,10 @@ struct dma_fence *dma_fence_get_stub(void) _fence_stub_ops, _fence_stub_lock, 0, 0); + + set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, + _fence_stub.flags); + dma_fence_signal_locked(_fence_stub); } spin_unlock(_fence_stub_lock); -- 2.25.1
[PATCH v3 1/6] dma-buf: Remove the signaled bit status check
Remove the signaled bit status check because it is returning
early when the fence is already signaled and
__dma_fence_enable_signaling is checking the status of signaled
bit again.
Signed-off-by: Arvind Yadav
---
Changes in v1, v2: This new patch was not part of previous series.
---
drivers/dma-buf/dma-fence.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 066400ed8841..64c99739ad23 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -601,9 +601,6 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence)
{
unsigned long flags;
- if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags))
- return;
-
spin_lock_irqsave(fence->lock, flags);
__dma_fence_enable_signaling(fence);
spin_unlock_irqrestore(fence->lock, flags);
--
2.25.1
[PATCH v3 0/6] dma-buf: Check status of enable-signaling bit on debug
Fence signaling must be enabled to make sure that the dma_fence_is_signaled() function ever returns true. Since drivers and implementations sometimes mess this up, this ensures correct behaviour when DEBUG_WW_MUTEX_SLOWPATH is used during debugging. This should make any implementation bugs resulting in not signaled fences much more obvious. Arvind Yadav (6): [PATCH v3 1/6] dma-buf: Remove the signaled bit status check [PATCH v3 2/6] dma-buf: set signaling bit for the stub fence [PATCH v3 3/6] dma-buf: Enable signaling on fence for selftests [PATCH v3 4/6] drm/amdgpu: Enable signaling on fence. [PATCH v3 5/6] drm/sched: Use parent fence instead of finished [PATCH v3 6/6] dma-buf: Check status of enable-signaling bit on debug drivers/dma-buf/dma-fence.c | 7 --- drivers/dma-buf/st-dma-fence-chain.c| 4 drivers/dma-buf/st-dma-fence-unwrap.c | 22 ++ drivers/dma-buf/st-dma-fence.c | 16 drivers/dma-buf/st-dma-resv.c | 10 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 ++ drivers/gpu/drm/scheduler/sched_main.c | 4 ++-- include/linux/dma-fence.h | 5 + 8 files changed, 65 insertions(+), 5 deletions(-) -- 2.25.1
Re: [PATCH v2 2/4] dma-buf: enable signaling for the stub fence on debug
On 9/6/2022 12:39 PM, Christian König wrote: Am 05.09.22 um 18:35 schrieb Arvind Yadav: Here's on debug enabling software signaling for the stub fence which is always signaled. This fence should enable software signaling otherwise the AMD GPU scheduler will cause a GPU reset due to a GPU scheduler cleanup activity timeout. Signed-off-by: Arvind Yadav --- Changes in v1 : 1- Addressing Christian's comment to remove unnecessary callback. 2- Replacing CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS. 3- The version of this patch is also changed and previously it was [PATCH 3/4] --- drivers/dma-buf/dma-fence.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 066400ed8841..2378b12538c4 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -27,6 +27,10 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled); static DEFINE_SPINLOCK(dma_fence_stub_lock); static struct dma_fence dma_fence_stub; +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH +static bool __dma_fence_enable_signaling(struct dma_fence *fence); +#endif + I would rename the function to something like dma_fence_enable_signaling_locked(). And please don't add any #ifdef if it isn't absolutely necessary. This makes the code pretty fragile. /* * fence context counter: each execution context should have its own * fence context, this allows checking if fences belong to the same @@ -136,6 +140,9 @@ struct dma_fence *dma_fence_get_stub(void) _fence_stub_ops, _fence_stub_lock, 0, 0); +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH + __dma_fence_enable_signaling(_fence_stub); +#endif Alternatively in this particular case you could just set the bit manually here since this is part of the dma_fence code anyway. Christian. As per per review comment. I will set the bit manually. ~arvind dma_fence_signal_locked(_fence_stub); } spin_unlock(_fence_stub_lock);
[git pull] drm fixes for 6.0-rc5
Hi Linus, >From a train in the Irish countryside, regular drm fixes for 6.0-rc5. This is mostly amdgpu/amdkfd and i915 fixes, then one panfrost, one ttm and one edid fix. Nothing too major going on. Hopefully a quiet week next week for LPC. Dave. drm-fixes-2022-09-10: drm fixes for 6.0-rc5 edid: - Fix EDID 1.4 range-descriptor parsing ttm: - Fix ghost-object bulk moves i915: - Fix MIPI sequence block copy from BIOS' table - Fix PCODE min freq setup when GuC's SLPC is in use - Implement Workaround for eDP - Fix has_flat_ccs selection for DG1 amdgpu: - Firmware header fix - SMU 13.x fix - Debugfs memory leak fix - NBIO 7.7 fix - Firmware memory leak fix amdkfd: - Debug output fix panfrost: - Fix devfreq OPP The following changes since commit 7e18e42e4b280c85b76967a9106a13ca61c16179: Linux 6.0-rc4 (2022-09-04 13:10:01 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-09-10 for you to fetch changes up to 2edb79a5fb303dff577d6a0c7d571c3bab1d1455: Merge tag 'drm-intel-fixes-2022-09-08' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2022-09-10 01:42:47 +1000) drm fixes for 6.0-rc5 edid: - Fix EDID 1.4 range-descriptor parsing ttm: - Fix ghost-object bulk moves i915: - Fix MIPI sequence block copy from BIOS' table - Fix PCODE min freq setup when GuC's SLPC is in use - Implement Workaround for eDP - Fix has_flat_ccs selection for DG1 amdgpu: - Firmware header fix - SMU 13.x fix - Debugfs memory leak fix - NBIO 7.7 fix - Firmware memory leak fix amdkfd: - Debug output fix panfrost: - Fix devfreq OPP Chengming Gui (1): drm/amd/amdgpu: add rlc_firmware_header_v2_4 to amdgpu_firmware_header Christian König (1): drm/ttm: cleanup the resource of ghost objects after locking them Clément Péron (1): drm/panfrost: devfreq: set opp to the recommended one to configure regulator Dave Airlie (3): Merge tag 'amd-drm-fixes-6.0-2022-09-07' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Merge tag 'drm-misc-fixes-2022-09-08' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'drm-intel-fixes-2022-09-08' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Evan Quan (1): drm/amd/pm: add missing SetMGpuFanBoostLimitRpm mapping for SMU 13.0.7 Greg Kroah-Hartman (1): drm/amd/display: fix memory leak when using debugfs_lookup() Guchun Chen (1): drm/amdgpu: prevent toc firmware memory leak Matthew Auld (1): drm/i915: consider HAS_FLAT_CCS() in needs_ccs_pages Rodrigo Vivi (1): drm/i915/slpc: Let's fix the PCODE min freq table setup for SLPC Ville Syrjälä (3): drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets drm/i915/bios: Copy the whole MIPI sequence block drm/i915: Implement WaEdpLinkRateDataReload Yifan Zhang (2): drm/amdkfd: print address in hex format rather than decimal drm/amdgpu: correct doorbell range/size value for CSDMA_DOORBELL_RANGE ZhenGuo Yin (1): drm/ttm: update bulk move object of ghost BO drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 7 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 1 + drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c | 6 --- .../drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 1 + .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 1 + drivers/gpu/drm/drm_debugfs.c | 4 +- drivers/gpu/drm/drm_edid.c | 24 --- drivers/gpu/drm/i915/display/intel_bios.c | 7 +++ .../gpu/drm/i915/display/intel_dp_link_training.c | 22 ++ drivers/gpu/drm/i915/gem/i915_gem_object.c | 3 ++ drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 2 +- drivers/gpu/drm/i915/gt/intel_llc.c| 19 drivers/gpu/drm/i915/gt/intel_rps.c| 50 ++ drivers/gpu/drm/i915/gt/intel_rps.h| 2 + drivers/gpu/drm/panfrost/panfrost_devfreq.c| 11 + drivers/gpu/drm/ttm/ttm_bo_util.c | 13 +++--- include/drm/drm_connector.h| 4 +- include/drm/drm_edid.h | 5 +++ 19 files changed, 149 insertions(+), 35 deletions(-)
Re: [PATCH] drm/rockchip: fix repeated words in comments
On Thu, 8 Sep 2022 20:36:16 +0800, wangjianli wrote: > Delete the redundant word 'in'. Applied, thanks! [1/1] drm/rockchip: fix repeated words in comments commit: fe53d167129e19ce01c056d85262427146cacf88 Best regards, -- Heiko Stuebner
Re: (subset) [PATCH v2 0/5] rockchip-dsi for rk3568
On Tue, 6 Sep 2022 12:48:18 -0500, Chris Morgan wrote: > This series adds support for the dsi and dphy controllers on the > Rockchip RK3568. I can confirm that for the Rockchip RK3568 this > current series DOES WORK now, but it requires rolling back clk changes > made for the HDMI driver. If the clock changes are not rolled back, the > image on the screen is shifted about 100 pixels to the right. > > Clk changes in question: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/rockchip/clk-rk3568.c?id=ff3187eabb5ce478d15b6ed62eb286756adefac3 > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/rockchip/clk-rk3568.c?id=6e69052f01d9131388cfcfaee929120118a267f4 > > [...] Applied, thanks! [1/5] dt-bindings: display: rockchip-dsi: add rk3568 compatible commit: 1c3b502e4327e8e24e617a6f922df72870c0cb5f [3/5] drm/rockchip: dsi: add rk3568 support commit: f3aaa6125b6f1532d3276d705b1a3791f18a872a Best regards, -- Heiko Stuebner
[PATCH] drm/msm/mdp5: fix kernel panic during shutdown
The kernel is panicking when rebooting on MSM8939:
# reboot -f
[ 87.280853] Unable to handle kernel write to read-only memory at
virtual address 88ed5810
...
snip
...
[ 87.445142] Call trace:
[ 87.452253] mutex_lock+0x1c/0x50
[ 87.454511] msm_drv_shutdown+0x28/0x40
[ 87.457984] platform_shutdown+0x28/0x40
[ 87.461629] device_shutdown+0x14c/0x240
[ 87.465796] __do_sys_reboot+0x180/0x274
[ 87.469703] __arm64_sys_reboot+0x28/0x3c
[ 87.473608] invoke_syscall+0x54/0x124
[ 87.477515] el0_svc_common.constprop.0+0x44/0xec
[ 87.481163] do_el0_svc+0x90/0xe0
[ 87.485934] el0_svc+0x50/0xa4
[ 87.489232] el0t_64_sync_handler+0x11c/0x150
[ 87.492185] el0t_64_sync+0x190/0x194
[ 87.496618] Code: f9800011 c85ffc03 ca010064 b564 (c8047c02)
[ 87.500264] ---[ end trace ]---
Segmentation fault
The issue comes from the fact that mdp5_init() is calling
platform_set_drvdata() and consequently overwriting the driver data
previously set by msm_drv_probe.
msm_drv_shutdown was casting the driver data as "struct msm_drm_private"
while it was actually a "struct mdp5_kms".
This commit fixes the issue by having mdp5_init() not override the
platform driver data, and instead use a series of
to_mdp5_kms(to_mdp_kms(priv->kms)) to retrieve the mdp5_kms from the
pdata.
Fixes: 54199009958f ("drm/msm: Fix shutdown")
Signed-off-by: Fabien Parent
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index d2a48caf9d27..17aeabeedfeb 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -634,7 +634,8 @@ static int mdp5_kms_init(struct drm_device *dev)
static void mdp5_destroy(struct platform_device *pdev)
{
- struct mdp5_kms *mdp5_kms = platform_get_drvdata(pdev);
+ struct msm_drm_private *priv = platform_get_drvdata(pdev);
+ struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
int i;
if (mdp5_kms->ctlm)
@@ -797,7 +798,8 @@ static int mdp5_init(struct platform_device *pdev, struct
drm_device *dev)
goto fail;
}
- platform_set_drvdata(pdev, mdp5_kms);
+ /* set uninit-ed kms */
+ priv->kms = _kms->base.base;
spin_lock_init(_kms->resource_lock);
@@ -890,13 +892,13 @@ static int mdp5_init(struct platform_device *pdev, struct
drm_device *dev)
if (ret)
goto fail;
- /* set uninit-ed kms */
- priv->kms = _kms->base.base;
-
return 0;
fail:
if (mdp5_kms)
mdp5_destroy(pdev);
+
+ priv->kms = NULL;
+
return ret;
}
@@ -956,7 +958,8 @@ static int mdp5_dev_remove(struct platform_device *pdev)
static __maybe_unused int mdp5_runtime_suspend(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
- struct mdp5_kms *mdp5_kms = platform_get_drvdata(pdev);
+ struct msm_drm_private *priv = platform_get_drvdata(pdev);
+ struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
DBG("");
@@ -966,7 +969,8 @@ static __maybe_unused int mdp5_runtime_suspend(struct
device *dev)
static __maybe_unused int mdp5_runtime_resume(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
- struct mdp5_kms *mdp5_kms = platform_get_drvdata(pdev);
+ struct msm_drm_private *priv = platform_get_drvdata(pdev);
+ struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
DBG("");
--
2.37.2
Re: [Intel-gfx] Developing a new backlight driver for specific OLED screen
On Fri, 09 Sep 2022, Rodrigo Vivi wrote: > On Fri, Sep 09, 2022 at 11:35:28AM +0200, Aurélien wrote: >>Hi, >>I hope this mailing-mist is the right place for this question. > > + dri-devel mailing list that looks more appropriated. > + Hans and Lyude who were recently working to standardize some of the > backlight stuff. > >>I would like to develop a new driver in order to manage backlight for a >>specific OLED display (Samsung one). For that propose I need to use the >>dpcd aux read and write functions. >>Since this driver is independent film the i915 driver I would like to >>develop an indémependant driver. >>So my question is: how can I use the i915 API (dpcd aux communications) >>outside from the driver and register the backlight sys entries like the >>i915 does (in order to have all the softwares which plays with the >>backlight working without modifying them) ? > > I don't believe you want to use the i915 API, but move the common functions > to the drm subsystem itself and then reuse as a drm device. Aurélien, are you aware of drivers/gpu/drm/display/drm_dp_helper.c and all the functions around struct dp_aux_backlight and struct drm_edp_backlight_info? Also see drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c. Does the display use some proprietary, non-VESA DPCD registers? There's already some of that in i915 for Intel proprietary interfaces. BR, Jani. > >>Many thanks for your answers >>-- >>Aurélien -- Jani Nikula, Intel Open Source Graphics Center
Re: [igt-dev] [PATCH i-g-t v2 3/4] lib/igt_kmod: add compatibility for KUnit
Hi Isabella,
On Monday, 29 August 2022 02:09:19 CEST Isabella Basso wrote:
> This adds functions for both executing the tests as well as parsing (K)TAP
> kmsg output, as per the KTAP spec [1].
>
> [1] https://www.kernel.org/doc/html/latest/dev-tools/ktap.html
>
> Signed-off-by: Isabella Basso
> ---
> lib/igt_kmod.c | 290 +
> lib/igt_kmod.h | 2 +
> 2 files changed, 292 insertions(+)
>
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 97cac7f5..93cdfcc5 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
>
> #include "igt_aux.h"
> #include "igt_core.h"
> @@ -32,6 +33,8 @@
> #include "igt_sysfs.h"
> #include "igt_taints.h"
>
> +#define BUF_LEN 4096
> +
> /**
> * SECTION:igt_kmod
> * @short_description: Wrappers around libkmod for module loading/unloading
> @@ -713,6 +716,293 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
> kmod_module_info_free_list(pre);
> }
>
> +/**
> + * lookup_value:
> + * @haystack: the string to search in
> + * @needle: the string to search for
> + *
> + * Returns: the value of the needle in the haystack, or -1 if not found.
> + */
> +static long lookup_value(const char *haystack, const char *needle)
> +{
> + const char *needle_rptr;
> + char *needle_end;
> + long num;
> +
> + needle_rptr = strcasestr(haystack, needle);
> +
> + if (needle_rptr == NULL)
> + return -1;
> +
> + /* skip search string and whitespaces after it */
> + needle_rptr += strlen(needle);
> +
> + num = strtol(needle_rptr, _end, 10);
> +
> + if (needle_rptr == needle_end)
> + return -1;
> +
> + if (num == LONG_MIN || num == LONG_MAX)
> + return 0;
> +
> + return num > 0 ? num : 0;
> +}
> +
> +static int find_next_tap_subtest(char *record, char *test_name,
> + bool is_subtest)
> +{
> + const char *name_lookup_str,
> + *lend, *version_rptr, *name_rptr;
> + long test_count;
> +
> + name_lookup_str = "test: ";
> +
> + version_rptr = strcasestr(record, "TAP version ");
> + name_rptr = strcasestr(record, name_lookup_str);
> +
> + /*
> + * total test count will almost always appear as 0..N at the beginning
> + * of a run, so we use it as indication of a run
> + */
> + test_count = lookup_value(record, "..");
> +
> + /* no count found, so this is probably not starting a (sub)test */
> + if (test_count < 0) {
> + if (name_rptr != NULL) {
> + if (test_name[0] == '\0')
> + strncpy(test_name,
> + name_rptr + strlen(name_lookup_str),
> + BUF_LEN);
> + else if (strcmp(test_name, name_rptr +
> strlen(name_lookup_str)) == 0)
> + return 0;
> + else
> + test_name[0] = '\0';
> +
> + }
> + return -1;
> + }
> +
> + /*
> + * "(K)TAP version XX" should be the first line on all (sub)tests as per
> + *
> https://www.kernel.org/doc/html/latest/dev-tools/ktap.html#version-lines
> + * but actually isn't, as it currently depends on whoever writes the
> + * test to print this info
> + */
> + if (version_rptr == NULL)
> + igt_info("Missing test version string\n");
> +
> + if (name_rptr == NULL) {
> + /* we have to keep track of the name string, as it might be
> + * contained in a line read previously */
> + if (test_name[0] == '\0') {
> + igt_info("Missing test name string\n");
> +
> + if (is_subtest)
> + igt_info("Running %ld subtests...\n",
> test_count);
> + else
> + igt_info("Running %ld tests...\n", test_count);
> + } else {
> + lend = strchrnul(test_name, '\n');
> +
> + if (*lend == '\0') {
> + if (is_subtest)
> + igt_info("Executing %ld subtests in:
> %s\n",
> + test_count, test_name);
> + else
> + igt_info("Executing %ld tests in: %s\n",
> + test_count, test_name);
> + return test_count;
> + }
> +
> + if (is_subtest)
> + igt_info("Executing %ld subtests in: %.*s\n",
> + test_count, (int)(lend - test_name),
> + test_name);
> + else
> + igt_info("Executing %ld tests in:
Re: [igt-dev] [PATCH i-g-t v2 2/4] lib/igt_kmod.c: check if module is builtin before attempting to unload it
On Monday, 29 August 2022 02:09:18 CEST Isabella Basso wrote:
> This change makes `igt_module_unload_r` safer as it checks whether the
s/igt_module_unload_r/igt_kmod_unload_r/
> module can be unloaded before attempting it.
>
> Signed-off-by: Isabella Basso
> ---
> lib/igt_kmod.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index bb6cb7bb..97cac7f5 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -256,6 +256,9 @@ static int igt_kmod_unload_r(struct kmod_module *kmod,
> unsigned int flags)
> struct kmod_list *holders, *pos;
> int err = 0;
>
> + if (kmod_module_get_initstate(kmod) == KMOD_MODULE_BUILTIN)
> + return err;
NIT: I think that return 0; would be more clear. Anyway,
Acked-by: Janusz Krzysztofik
> +
> holders = kmod_module_get_holders(kmod);
> kmod_list_foreach(pos, holders) {
> struct kmod_module *it = kmod_module_get_module(pos);
>
[PATCH] drm/hyperv: Add ratelimit on error message
Due to a full ring buffer, the driver may be unable to send updates to the Hyper-V host. But outputing the error message can make the problem worse because console output is also typically written to the frame buffer. Rate limiting the error message, also output the error code for additional diagnosability. Signed-off-by: Saurabh Sengar --- drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c index 76a182a..013a782 100644 --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c @@ -208,7 +208,7 @@ static inline int hyperv_sendpacket(struct hv_device *hdev, struct synthvid_msg VM_PKT_DATA_INBAND, 0); if (ret) - drm_err(>dev, "Unable to send packet via vmbus\n"); + drm_err_ratelimited(>dev, "Unable to send packet via vmbus; error %d\n", ret); return ret; } -- 1.8.3.1
Re: [igt-dev] [PATCH i-g-t v2 1/4] lib/igt_kmod: rename kselftest functions to ktest
On Monday, 29 August 2022 02:09:17 CEST Isabella Basso wrote:
> This aims at making IGT's structure more general to different kernel
> testing frameworks such as KUnit, as they use a lot of the same
> functionality.
>
> Signed-off-by: Isabella Basso
LGTM
Reviewed-by: Janusz Krzysztofik
> ---
> lib/igt_kmod.c | 22 +++---
> lib/igt_kmod.h | 12 ++--
> 2 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index bcf7dfeb..bb6cb7bb 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -718,8 +718,8 @@ static int open_parameters(const char *module_name)
> return open(path, O_RDONLY);
> }
>
> -int igt_kselftest_init(struct igt_kselftest *tst,
> -const char *module_name)
> +int igt_ktest_init(struct igt_ktest *tst,
> +const char *module_name)
> {
> int err;
>
> @@ -738,7 +738,7 @@ int igt_kselftest_init(struct igt_kselftest *tst,
> return 0;
> }
>
> -int igt_kselftest_begin(struct igt_kselftest *tst)
> +int igt_ktest_begin(struct igt_ktest *tst)
> {
> int err;
>
> @@ -753,7 +753,7 @@ int igt_kselftest_begin(struct igt_kselftest *tst)
> return 0;
> }
>
> -int igt_kselftest_execute(struct igt_kselftest *tst,
> +int igt_kselftest_execute(struct igt_ktest *tst,
> struct igt_kselftest_list *tl,
> const char *options,
> const char *result)
> @@ -791,13 +791,13 @@ int igt_kselftest_execute(struct igt_kselftest *tst,
> return err;
> }
>
> -void igt_kselftest_end(struct igt_kselftest *tst)
> +void igt_ktest_end(struct igt_ktest *tst)
> {
> kmod_module_remove_module(tst->kmod, KMOD_REMOVE_FORCE);
> close(tst->kmsg);
> }
>
> -void igt_kselftest_fini(struct igt_kselftest *tst)
> +void igt_ktest_fini(struct igt_ktest *tst)
> {
> free(tst->module_name);
> kmod_module_unref(tst->kmod);
> @@ -820,15 +820,15 @@ void igt_kselftests(const char *module_name,
> const char *result,
> const char *filter)
> {
> - struct igt_kselftest tst;
> + struct igt_ktest tst;
> IGT_LIST_HEAD(tests);
> struct igt_kselftest_list *tl, *tn;
>
> - if (igt_kselftest_init(, module_name) != 0)
> + if (igt_ktest_init(, module_name) != 0)
> return;
>
> igt_fixture
> - igt_require(igt_kselftest_begin() == 0);
> + igt_require(igt_ktest_begin() == 0);
>
> igt_kselftest_get_tests(tst.kmod, filter, );
> igt_subtest_with_dynamic(filter ?: "all") {
> @@ -847,9 +847,9 @@ void igt_kselftests(const char *module_name,
> }
>
> igt_fixture {
> - igt_kselftest_end();
> + igt_ktest_end();
> igt_require(!igt_list_empty());
> }
>
> - igt_kselftest_fini();
> + igt_ktest_fini();
> }
> diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
> index f98dd29f..ceb10cd0 100644
> --- a/lib/igt_kmod.h
> +++ b/lib/igt_kmod.h
> @@ -50,7 +50,7 @@ void igt_kselftests(const char *module_name,
> const char *result_option,
> const char *filter);
>
> -struct igt_kselftest {
> +struct igt_ktest {
> struct kmod_module *kmod;
> char *module_name;
> int kmsg;
> @@ -63,19 +63,19 @@ struct igt_kselftest_list {
> char param[];
> };
>
> -int igt_kselftest_init(struct igt_kselftest *tst,
> +int igt_ktest_init(struct igt_ktest *tst,
> const char *module_name);
> -int igt_kselftest_begin(struct igt_kselftest *tst);
> +int igt_ktest_begin(struct igt_ktest *tst);
>
> void igt_kselftest_get_tests(struct kmod_module *kmod,
>const char *filter,
>struct igt_list_head *tests);
> -int igt_kselftest_execute(struct igt_kselftest *tst,
> +int igt_kselftest_execute(struct igt_ktest *tst,
> struct igt_kselftest_list *tl,
> const char *module_options,
> const char *result);
>
> -void igt_kselftest_end(struct igt_kselftest *tst);
> -void igt_kselftest_fini(struct igt_kselftest *tst);
> +void igt_ktest_end(struct igt_ktest *tst);
> +void igt_ktest_fini(struct igt_ktest *tst);
>
> #endif /* IGT_KMOD_H */
>
Re: [PATCH v2 2/2] drm/etnaviv: fix power register offset on GC300
Hi Lucas,
On 9/9/2022 1:48 AM, Lucas Stach wrote:
@@ -83,10 +83,15 @@ static void etnaviv_core_dump_registers(struct
core_dump_iterator *iter,
{
struct etnaviv_dump_registers *reg = iter->data;
unsigned int i;
+ u32 addr;
for (i = 0; i < ARRAY_SIZE(etnaviv_dump_registers); i++, reg++) {
- reg->reg = cpu_to_le32(etnaviv_dump_registers[i]);
- reg->value = cpu_to_le32(gpu_read(gpu,
etnaviv_dump_registers[i]));
+ addr = etnaviv_dump_registers[i];
+ if (addr >= VIVS_PM_POWER_CONTROLS &&
+ addr <= VIVS_PM_PULSE_EATER)
+ addr = gpu_fix_power_address(gpu, addr);
+ reg->reg = cpu_to_le32(addr);
As the dumpdecoder tool would then need to reverse this address offset,
I think it would be preferable to keep writing the canonical (not fixed
up) register address into the dump. That way only the kernel needs to
know about this special offset on GC300.
Ahh, I had no idea about how it worked on that side of things. Makes
complete sense. Thanks for reviewing! Will submit V3 with this and
everything else you mentioned fixed.
Doug
Re: [PATCH v4 4/8] drm/vc4: hdmi: Simplify the hotplug handling
On Fri, Sep 09, 2022 at 04:36:44PM +0200, Maxime Ripard wrote:
> Hi Ville
>
> Thanks for your review
>
> On Mon, Sep 05, 2022 at 08:38:11PM +0300, Ville Syrjälä wrote:
> > On Mon, Aug 29, 2022 at 03:47:27PM +0200, Maxime Ripard wrote:
> > > Our detect callback has a bunch of operations to perform depending on
> > > the current and last status of the connector, such a setting the CEC
> > > physical address or enabling the scrambling again.
> > >
> > > This is currently dealt with a bunch of if / else statetements that make
> > > it fairly difficult to read and extend.
> > >
> > > Let's move all that logic to a function of its own.
> > >
> > > Signed-off-by: Maxime Ripard
> > > ---
> > > drivers/gpu/drm/vc4/vc4_hdmi.c | 66 ++
> > > 1 file changed, 44 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > index b786645eaeb7..9fad57ebdd11 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > @@ -273,17 +273,53 @@ static void vc4_hdmi_cec_update_clk_div(struct
> > > vc4_hdmi *vc4_hdmi) {}
> > >
> > > static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
> > >
> > > +static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> > > + enum drm_connector_status status)
> > > +{
> > > + struct drm_connector *connector = _hdmi->connector;
> > > + struct edid *edid;
> > > +
> > > + /*
> > > + * NOTE: This function should really be called with
> > > + * vc4_hdmi->mutex held, but doing so results in reentrancy
> > > + * issues since cec_s_phys_addr_from_edid might call
> > > + * .adap_enable, which leads to that funtion being called with
> > > + * our mutex held.
> > > + *
> > > + * Concurrency isn't an issue at the moment since we don't share
> > > + * any state with any of the other frameworks so we can ignore
> > > + * the lock for now.
> > > + */
> > > +
> > > + if (status == connector->status)
> > > + return;
> >
> > This looks to have a change in behaviour to not call
> > vc4_hdmi_enable_scrambling() unless a change in the
> > connector status was detected. The previous code called
> > it regarless.
>
> Yeah, good point
>
> > When I was doing the i915 stuff I did have a sink that
> > left hpd asserted while turned off, and when powering
> > back up it instead generated a pulse on the hpd line.
> > Not sure if such a pulse is always slow enough that
> > you can reasonably guarantee a detection of both edges...
> >
> > Apart from that (and the cec locking mess that I dared
> > not even look at) the rest of the series looks OK to me.
>
> Can I add your R-B and remove the check you mentioned above while
> applying, or would you prefer that I send a new version?
Nah. Feel free slap on
Reviewed-by: Ville Syrjälä
to the lot if you don't think a resend is needed otherwise.
--
Ville Syrjälä
Intel
Re: [igt-dev] [PATCH i-g-t v2 0/4] Add support for KUnit tests
Hi Isabella, On Monday, 29 August 2022 02:09:16 CEST Isabella Basso wrote: > This patch series was first developed as part of the LKCamp hackathon > that happened last year[1], mainly focusing on refactoring DRM tests to > use KUnit. > > KUnit[2][3] is a unified test framework that provides helper tools, > simplifying their development and execution. Using an x86-64 machine > it's possible to run tests in the host's kernel natively using user-mode > Linux[4] (aka UML), which simplifies usage in a wide variety of > scenarios, including integration to CI. > > As the tool's adoption widens into graphics testing territory, I and > LKCamp members figured it would be important to support it in IGT, as > it's a core tool for GPU drivers maintainers. > > I have then added KUnit support into IGT mainly following the KTAP > specs, and it can be tested using patch 4/4 in this series together with > a DRM selftests patch series available at [5]. CI pre-merge results indicate that your new versions of tests were not able to load kunit module, then didn't actually execute any kunit tests, but returned SUCCESS. That's probably not what we expect from IGT tests. https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7698/shard-rkl-4/igt@drm_buddy.html https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7698/shard-rkl-1/igt@drm_mm.html https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7698/shard-rkl-5/igt@kms_selftest.html Thanks, Janusz > > Changes since v1: > - Major rework of parsing function structure: > - It is not longer recursive > - Adapt kselftests functions and structs to be used with KUnit > - Switch DRM selftests to KUnit parsing as they're updated in the kernel > - Replace AMD KUnit tests by DRM selftests > > [1]: https://groups.google.com/g/kunit-dev/c/YqFR1q2uZvk/m/IbvItSfHBAAJ > [2]: https://kunit.dev > [3]: https://docs.kernel.org/dev-tools/kunit/index.html > [4]: http://user-mode-linux.sourceforge.net > [5]: https://lore.kernel.org/all/20220708203052.236290-1-maira.ca...@usp.br/ > > Isabella Basso (4): > lib/igt_kmod: rename kselftest functions to ktest > lib/igt_kmod.c: check if module is builtin before attempting to unload > it > lib/igt_kmod: add compatibility for KUnit > tests: DRM selftests: switch to KUnit > > lib/igt_kmod.c | 315 +-- > lib/igt_kmod.h | 14 +- > tests/drm_buddy.c| 7 +- > tests/drm_mm.c | 7 +- > tests/kms_selftest.c | 12 +- > 5 files changed, 329 insertions(+), 26 deletions(-) > >
[PATCH] drm/hyperv: Don't rely on screen_info.lfb_base for Gen1 VMs
hyperv_setup_vram tries to remove conflicting framebuffer based on
'screen_info'. As observed in past due to some bug or wrong setting
in grub, the 'screen_info' fields may not be set for Gen1, and in such
cases drm_aperture_remove_conflicting_framebuffers will not do anything
useful.
For Gen1 VMs, it should always be possible to get framebuffer
conflict removed using PCI device instead.
Fixes: a0ab5abced55 ("drm/hyperv : Removing the restruction of VRAM allocation
with PCI bar size")
Signed-off-by: Saurabh Sengar
---
drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index 6d11e7938c83..b0cc974efa45 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -73,12 +73,28 @@ static int hyperv_setup_vram(struct hyperv_drm_device *hv,
struct hv_device *hdev)
{
struct drm_device *dev = >dev;
+ struct pci_dev *pdev;
int ret;
- drm_aperture_remove_conflicting_framebuffers(screen_info.lfb_base,
-screen_info.lfb_size,
-false,
-_driver);
+ if (efi_enabled(EFI_BOOT)) {
+
drm_aperture_remove_conflicting_framebuffers(screen_info.lfb_base,
+
screen_info.lfb_size,
+false,
+_driver);
+ } else {
+ pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
+ if (!pdev) {
+ drm_err(dev, "Unable to find PCI Hyper-V video\n");
+ return -ENODEV;
+ }
+
+ ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev,
_driver);
+ pci_dev_put(pdev);
+ if (ret) {
+ drm_err(dev, "Not able to remove boot fb\n");
+ return ret;
+ }
+ }
hv->fb_size = (unsigned long)hv->mmio_megabytes * 1024 * 1024;
--
2.34.1
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
Hi, On 9/9/22 15:39, Simon Ser wrote: > On Friday, September 9th, 2022 at 12:12, Hans de Goede > wrote: > >> Phase 3: Deprecate /sys/class/backlight uAPI >> >> >> Once most userspace has moved over to using the new drm_connector >> brightness props, a Kconfig option can be added to stop exporting >> the backlight-devices under /sys/class/backlight. The plan is to >> just disable the sysfs interface and keep the existing backlight-device >> internal kernel abstraction as is, since some abstraction for (non GPU >> native) backlight devices will be necessary regardless. >> >> It is unsure if we will ever be able to do this. For example people using >> non fully integrated desktop environments like e.g. sway often use custom >> scripts binded to hotkeys to get functionality like the brightness >> up/down keyboard hotkeys changing the brightness. This typically involves >> e.g. the xbacklight utility. >> >> Even if the xbacklight utility is ported to use kms with the new connector >> object brightness properties then this still will not work because >> changing the properties will require drm-master rights and e.g. sway will >> already hold those. > > I replied to this here in another thread [1]. > > tl;dr I think it would be fine even for Sway-like compositors. Ok, that is good to know. > (Also note the utilities used right now are not xbacklight, but > brightnessctl/light/brillo/etc [2]) Ah I thought that xbacklight got patched at one point to support the sysfs API, but I see now that instead alternative utilities have popped up. Regards, Hans > > [1]: > https://lore.kernel.org/dri-devel/bZJU9OkYWFyaLHVa4XUE4d5iBTPFXBRyPe1wMd_ztKh5VBMu-EDNGoUDpvwtFn_u9-JMvN8QmIZVS3pzMZM_hZTkTCA9gOBnCGXc5HFmsnc=@emersion.fr/ > [2]: https://github.com/swaywm/sway/wiki#xbacklight > >> The drm_connector brightness properties >> === >> >> The new uAPI for this consists of 2 properties: >> >> 1. "display brightness": rw 0-int32_max property controlling the brightness >> setting >> of the connected display. The actual maximum of this will be less then >> int32_max and is given in "display brightness max". >> >> Unlike the /sys/class/backlight/foo/brightness this brightness property >> has a clear definition for the value 0. The kernel must ensure that 0 >> means minimum brightness (so 0 should never turn the backlight off). >> If necessary the kernel must enforce a minimum value by adding >> an offset to the value seen in the property to ensure this behavior. >> >> For example if necessary the driver must clamp 0-255 to 10-255, which then >> becomes 0-245 on the brightness property, adding 10 internally to writes >> done to the brightness property. This adding of an extra offset when >> necessary must only be done on the brightness property, >> the /sys/class/backlight interface should be left unchanged to not break >> userspace which may rely on 0 = off on some systems. >> >> Note amdgpu already does something like this even for /sys/class/backlight, >> see the use of AMDGPU_DM_DEFAULT_MIN_BACKLIGHT in amdgpu. >> >> Also whenever possible the kernel must ensure that the brightness range >> is in perceived brightness, but this cannot always be guaranteed. >> >> >> 2. "display brightness max": ro 0-int32_max property giving the actual >> maximum >> of the display's brightness setting. This will report 0 when brightness >> control is not available. >> >> The value of "display brightness max" may change at runtime, either by >> a legacy drivers/platform/x86 backlight driver loading after the drm >> driver has loaded; or when plugging in a monitor which allows brightness >> control over DDC/CI. In both these cases the max value will change from 0 >> to the actual max value, indicating backlight control has become available >> on this connector. > > The kernel will need to ensure that a hotplug uevent is sent to > user-space at this point. Otherwise user-space has no way to figure out > that the prop has changed. > > Overall this all looks very solid to me! > > Simon >
Re: Meeting (BOF) at Plumbers Dublin to discuss backlight brightness as connector object property RFC?
On Fri, 9 Sept 2022 at 12:50, Simon Ser wrote: > On Friday, September 9th, 2022 at 12:23, Hans de Goede < > hdego...@redhat.com> wrote: > > "people using > > non fully integrated desktop environments like e.g. sway often use custom > > scripts binded to hotkeys to get functionality like the brightness > > up/down keyboard hotkeys changing the brightness. This typically involves > > e.g. the xbacklight utility. > > > > Even if the xbacklight utility is ported to use kms with the new > connector > > object brightness properties then this still will not work because > > changing the properties will require drm-master rights and e.g. sway will > > already hold those." > > I don't think this is a good argument. Sway (which I'm a maintainer of) > can add a command to change the backlight, and then users can bind their > keybinding to that command. This is not very different from e.g. a > keybind to switch on/off a monitor. > > We can also standardize a protocol to change the backlight across all > non-fully-integrated desktop environments (would be a simple addition > to output-power-management [1]), so that a single tool can work for > multiple compositors. Yeah, I mean, as one of the main people arguing that non-fully-integrated desktops are not the design we want, I agree with Simon. Cheers, Daniel
Re: [PATCH v4 4/8] drm/vc4: hdmi: Simplify the hotplug handling
Hi Ville
Thanks for your review
On Mon, Sep 05, 2022 at 08:38:11PM +0300, Ville Syrjälä wrote:
> On Mon, Aug 29, 2022 at 03:47:27PM +0200, Maxime Ripard wrote:
> > Our detect callback has a bunch of operations to perform depending on
> > the current and last status of the connector, such a setting the CEC
> > physical address or enabling the scrambling again.
> >
> > This is currently dealt with a bunch of if / else statetements that make
> > it fairly difficult to read and extend.
> >
> > Let's move all that logic to a function of its own.
> >
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/gpu/drm/vc4/vc4_hdmi.c | 66 ++
> > 1 file changed, 44 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index b786645eaeb7..9fad57ebdd11 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -273,17 +273,53 @@ static void vc4_hdmi_cec_update_clk_div(struct
> > vc4_hdmi *vc4_hdmi) {}
> >
> > static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
> >
> > +static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> > + enum drm_connector_status status)
> > +{
> > + struct drm_connector *connector = _hdmi->connector;
> > + struct edid *edid;
> > +
> > + /*
> > +* NOTE: This function should really be called with
> > +* vc4_hdmi->mutex held, but doing so results in reentrancy
> > +* issues since cec_s_phys_addr_from_edid might call
> > +* .adap_enable, which leads to that funtion being called with
> > +* our mutex held.
> > +*
> > +* Concurrency isn't an issue at the moment since we don't share
> > +* any state with any of the other frameworks so we can ignore
> > +* the lock for now.
> > +*/
> > +
> > + if (status == connector->status)
> > + return;
>
> This looks to have a change in behaviour to not call
> vc4_hdmi_enable_scrambling() unless a change in the
> connector status was detected. The previous code called
> it regarless.
Yeah, good point
> When I was doing the i915 stuff I did have a sink that
> left hpd asserted while turned off, and when powering
> back up it instead generated a pulse on the hpd line.
> Not sure if such a pulse is always slow enough that
> you can reasonably guarantee a detection of both edges...
>
> Apart from that (and the cec locking mess that I dared
> not even look at) the rest of the series looks OK to me.
Can I add your R-B and remove the check you mentioned above while
applying, or would you prefer that I send a new version?
Thanks!
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v2 4/5] phy/rockchip: inno-dsidphy: Add support for rk3568
Am Dienstag, 6. September 2022, 19:48:22 CEST schrieb Chris Morgan: > From: Chris Morgan > > Add support for the Rockchip RK3568 DSI-DPHY. Registers were taken from > the BSP kernel driver and wherever possible cross referenced with the > TRM. With the amount of refactoring done below, I'd expect a bit more introductory text here ;-) I.e. about older variants of the phy only supporting 1GHz rates and newer ones supporting up to 2.5GHz and that you refactor some things to make both variants work. > > Signed-off-by: Chris Morgan > --- > .../phy/rockchip/phy-rockchip-inno-dsidphy.c | 204 ++ > 1 file changed, 158 insertions(+), 46 deletions(-) > > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c > b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c > index 630e01b5c19b..2c5847faff63 100644 > --- a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c > +++ b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c > @@ -84,9 +84,25 @@ > #define DATA_LANE_0_SKEW_PHASE_MASK GENMASK(2, 0) > #define DATA_LANE_0_SKEW_PHASE(x)UPDATE(x, 2, 0) > /* Analog Register Part: reg08 */ > +#define PLL_POST_DIV_ENABLE_MASK BIT(5) > +#define PLL_POST_DIV_ENABLE BIT(5) > #define SAMPLE_CLOCK_DIRECTION_MASK BIT(4) > #define SAMPLE_CLOCK_DIRECTION_REVERSE BIT(4) > #define SAMPLE_CLOCK_DIRECTION_FORWARD 0 > +#define LOWFRE_EN_MASK BIT(5) PLL_POST_DIR_ENABLE above also is BIT(5) ... is this correct? otherwise the changes look great. Heiko
Re: [PATCH v2 4/4] vfio/pci: Allow MMIO regions to be exported through dma-buf
On Fri, Sep 09, 2022 at 06:24:35AM -0700, Christoph Hellwig wrote: > On Wed, Sep 07, 2022 at 01:12:52PM -0300, Jason Gunthorpe wrote: > > The PCI offset is some embedded thing - I've never seen it in a server > > platform. > > That's not actually true, e.g. some power system definitively had it, > althiugh I don't know if the current ones do. I thought those were all power embedded systems. > There is a reason why we have these proper APIs and no one has any > business bypassing them. Yes, we should try to support these things, but you said this patch didn't work and wasn't tested - that is not true at all. And it isn't like we have APIs just sitting here to solve this specific problem. So lets make something. > > So, would you be OK with this series if I try to make a dma_map_p2p() > > that resolves the offset issue? > > Well, if it also solves the other issue of invalid scatterlists leaking > outside of drm we can think about it. The scatterlist stuff has already leaked outside of DRM anyhow. Again, I think it is very problematic to let DRM get away with things and then insist all the poor non-DRM people be responsible to clean up their mess. I'm skeptical I can fix AMD GPU, but I can try to create a DMABUF op that returns something that is not a scatterlist and teach RDMA to use it. So at least the VFIO/RDMA part can avoid the scatter list abuse. I expected to need non-scatterlist for iommufd anyhow. Coupled with a series to add some dma_map_resource_pci() that handles the PCI_P2PDMA_MAP_BUS_ADDR and the PCI offset, would it be an agreeable direction? > Take a look at iommu_dma_map_sg and pci_p2pdma_map_segment to see how > this is handled. So there is a bug in all these DMABUF implementations, they do ignore the PCI_P2PDMA_MAP_BUS_ADDR "distance type". This isn't a real-world problem for VFIO because VFIO is largely incompatible with the non-ACS configuration that would trigger PCI_P2PDMA_MAP_BUS_ADDR, and explains why we never saw any problem. All our systems have ACS turned on so we can use VFIO. I'm unclear how Habana or AMD have avoided a problem here.. This is much more serious than the pci offset in my mind. Thanks, Jason
Re: [PATCH 2/3] pci_ids: Add Microsoft PCI Vendor ID, and remove redundant definitions
eahar...@linux.microsoft.com writes:
> From: Easwar Hariharan
>
> Move the Microsoft PCI Vendor ID from the various drivers to the pci_ids
> file
>
> Signed-off-by: Easwar Hariharan
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 1 -
> drivers/net/ethernet/microsoft/mana/gdma_main.c | 4
> drivers/video/fbdev/hyperv_fb.c | 3 ---
> include/linux/pci_ids.h | 2 ++
> 4 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 6d11e79..61083c7 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -23,7 +23,6 @@
> #define DRIVER_MAJOR 1
> #define DRIVER_MINOR 0
>
> -#define PCI_VENDOR_ID_MICROSOFT 0x1414
> #define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
>
> DEFINE_DRM_GEM_FOPS(hv_fops);
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 5f92401..00d8198 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1465,10 +1465,6 @@ static void mana_gd_shutdown(struct pci_dev *pdev)
> pci_disable_device(pdev);
> }
>
> -#ifndef PCI_VENDOR_ID_MICROSOFT
> -#define PCI_VENDOR_ID_MICROSOFT 0x1414
> -#endif
> -
> static const struct pci_device_id mana_id_table[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_PF_DEVICE_ID) },
> { PCI_DEVICE(PCI_VENDOR_ID_MICROSOFT, MANA_VF_DEVICE_ID) },
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index 886c564..a502c80 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -58,7 +58,6 @@
>
> #include
>
> -
> /* Hyper-V Synthetic Video Protocol definitions and structures */
> #define MAX_VMBUS_PKT_SIZE 0x4000
>
> @@ -74,10 +73,8 @@
> #define SYNTHVID_DEPTH_WIN8 32
> #define SYNTHVID_FB_SIZE_WIN8 (8 * 1024 * 1024)
>
> -#define PCI_VENDOR_ID_MICROSOFT 0x1414
> #define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
>
> -
> enum pipe_msg_type {
> PIPE_MSG_INVALID,
> PIPE_MSG_DATA,
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 6feade6..c008fda 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2079,6 +2079,8 @@
> #define PCI_DEVICE_ID_ICE_1712 0x1712
> #define PCI_DEVICE_ID_VT1724 0x1724
>
> +#define PCI_VENDOR_ID_MICROSOFT 0x1414
> +
> #define PCI_VENDOR_ID_OXSEMI 0x1415
> #define PCI_DEVICE_ID_OXSEMI_12PCI8400x8403
> #define PCI_DEVICE_ID_OXSEMI_PCIe840 0xC000
I've sent a similar patch recently:
https://lore.kernel.org/linux-hyperv/20220827130345.1320254-2-vkuzn...@redhat.com/
which Wei has already queued to hyperv/fixes. Moving
PCI_DEVICE_ID_MICROSOFT_MANA_PF/VF definitions to 'pci_ids.h' does make
sense but please rebase first.
--
Vitaly
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Friday, September 9th, 2022 at 15:53, Pekka Paalanen wrote: > On Fri, 09 Sep 2022 13:39:37 + > Simon Ser cont...@emersion.fr wrote: > > > On Friday, September 9th, 2022 at 12:12, Hans de Goede hdego...@redhat.com > > wrote: > > > > > Phase 3: Deprecate /sys/class/backlight uAPI > > > > > > > > > Once most userspace has moved over to using the new drm_connector > > > brightness props, a Kconfig option can be added to stop exporting > > > the backlight-devices under /sys/class/backlight. The plan is to > > > just disable the sysfs interface and keep the existing backlight-device > > > internal kernel abstraction as is, since some abstraction for (non GPU > > > native) backlight devices will be necessary regardless. > > > > > > It is unsure if we will ever be able to do this. For example people using > > > non fully integrated desktop environments like e.g. sway often use custom > > > scripts binded to hotkeys to get functionality like the brightness > > > up/down keyboard hotkeys changing the brightness. This typically involves > > > e.g. the xbacklight utility. > > > > > > Even if the xbacklight utility is ported to use kms with the new connector > > > object brightness properties then this still will not work because > > > changing the properties will require drm-master rights and e.g. sway will > > > already hold those. > > > > I replied to this here in another thread 1. > > > > tl;dr I think it would be fine even for Sway-like compositors. > > Furthermore, if other compositors are like Weston in their KMS state > handling, and do not track which property has already been programmed > to KMS and which hasn't, and instead just smash all KMS properties > every update anyway (it's also great for debugging, you always have the > full state in flight), anything changed via sysfs will be immediately > reverted. > > Therefore I think there is a high probability that the external or > sysfs controls just naturally stop working anyway, even if the kernel > does not remove them first. Ah yes, that's a good point. And this wouldn't be a kernel regression, it would be user-space like Sway or Weston taking the decision to use the new uAPI in a way which breaks the sysfs interface. (User-space could also take the decision to _not_ break the sysfs interface, by implementing a simple "dirty" flag.)
Re: [PATCH v2 10/41] drm/modes: Add a function to generate analog display modes
On Wed, Sep 07, 2022 at 11:31:21PM +0200, Mateusz Kwiatkowski wrote: > The "canonical" modelines (at least for vc4's VEC, see the notes below): > > - (vfp==4, vsync==6, vbp==39) for 576i > - (vfp==7, vsync==6, vbp==32) for 480i > - (vfp==5, vsync==6, vbp==28) for 486i (full frame NTSC as originally > specified) It's not clear to me either how you come up with those timings? Maxime signature.asc Description: PGP signature
Re: [PATCH v2 2/5] dt-bindings: phy-rockchip-inno-dsidphy: add compatible for rk3568
Am Dienstag, 6. September 2022, 19:48:20 CEST schrieb Chris Morgan: > From: Chris Morgan > > Add a compatible string for the rk3568 dsi-dphy. > > Signed-off-by: Chris Morgan Reviewed-by: Heiko Stuebner > --- > .../devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git > a/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml > b/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml > index 8a3032a3bd73..5c35e5ceec0b 100644 > --- a/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml > +++ b/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml > @@ -18,6 +18,7 @@ properties: >- rockchip,px30-dsi-dphy >- rockchip,rk3128-dsi-dphy >- rockchip,rk3368-dsi-dphy > + - rockchip,rk3568-dsi-dphy > >reg: > maxItems: 1 >
Re: [PATCH v2 10/41] drm/modes: Add a function to generate analog display modes
Hi, On Wed, Sep 07, 2022 at 11:31:21PM +0200, Mateusz Kwiatkowski wrote: > W dniu 7.09.2022 o 16:34, Maxime Ripard pisze: > > On Mon, Sep 05, 2022 at 06:44:42PM +0200, Mateusz Kwiatkowski wrote: > >> Hi Maxime, > >> > >> W dniu 5.09.2022 o 15:37, Maxime Ripard pisze: > > + vfp = vfp_min + (porches_rem / 2); > > + vbp = porches - vfp; > > Relative position of the vertical sync within the VBI effectively moves > the > image up and down. Adding that (porches_rem / 2) moves the image up off > center > by that many pixels. I'd keep the VFP always at minimum to keep the image > centered. > >>> > >>> And you would increase the back porch only then? > >> > >> Well, increasing vbp only gives a centered image with the default 480i/576i > >> resolutions. However, only ever changing vbp will cause the image to be > >> always > >> at the bottom of the screen when the active line count is decreased (e.g. > >> setting the resolution to 720x480 but for 50Hz "PAL" - like many game > >> consoles > >> did back in the day). > >> > >> I believe that the perfect solution would: > >> > >> - Use the canonical / standard-defined blanking line counts for the > >> standard > >> vertical resolutions (480/486/576) > >> - Increase vfp and vbp from there by the same number if a smaller number of > >> active lines is specified, so that the resulting image is centered > >> - Likewise, decrease vfp and vbp by the same number if the active line > >> number > >> is larger and there is still leeway (this should allow for seamless > >>handling > >> of 480i vs. 486i for 60 Hz "NTSC") > > > > I'm not sure I understand how that's any different than the code you > > initially commented on. > > > > I would start by taking the entire blanking area, remove the sync > > period. We only have the two porches now, and I'm starting from the > > minimum, adding as many pixels in both (unless it's not an even number, > > in which case the backporch will have the extra pixel). > > > > Isn't it the same thing? > > [...] > > Unless you only want me to consider the front porch maximum? > > I think you're confusing the "post-equalizing pulses" with the "vertical back > porch" a little bit. The "vertical back porch" includes both the > post-equalizing > pulses and the entire rest of the VBI period, for the standard resolutions at > least. > > The "canonical" modelines (at least for vc4's VEC, see the notes below): > > - (vfp==4, vsync==6, vbp==39) for 576i > - (vfp==7, vsync==6, vbp==32) for 480i > - (vfp==5, vsync==6, vbp==28) for 486i (full frame NTSC as originally > specified) > > The numbers for vfp don't exactly match the theoretical values, because: > > - VEC actually adds a half-line pulse on top of VFP_ODD, and in the 625-line > mode also on top of VFP_EVEN (not always, but let's not dwell too much) > - Conversely, VEC subtracts the half-line pulse from VSYNC_ODD and VSYNC_EVEN > in the 625-line mode > - SMPTE S170M (see https://www.itu.int/rec/R-REC-BT.1700-0-200502-I/en) > defines > that active picture for NTSC is on lines 21-263 and 283-525; 263 and 283 are > half-lines that are represented as full lines in the "486i" spec It's going to be a bit difficult to match that into a drm_display_mode, since as far I understand it, all the timings are the sum of the timings of both fields in interlaced. I guess we'll have to be close enough. > - SMPTE 314M, which is the spec for DV, defines the 480 active lines as lines > 23-262 and 285-524; see Table 20 on page 26 in > > https://last.hit.bme.hu/download/firtha/video/SMPTE/SMPTE-314M%20DV25-50.pdf; > this means that the standard 480i frame shaves off four topmost and two > bottommost lines (2 and 1 per field) of the 486i full frame I'm still struggling a bit to match that into front porch, sync period and back porch. I guess the sync period is easy since it's pretty much fixed. That line 0-23 is the entire blanking area though, right? > Note that the half-line pulses in vfp/vsync may be generated in a different > way > on encoders other than vc4's VEC. Maybe we should define some concrete > semantics for vfp/vsync in analog TV modes, and compensate for that in the > drivers. But anyway, that's a separate issue. > > My point is that, to get a centered image, you can then proportionately add > values to those "canonical" vfp/vbp values. For example if someone specifies > 720x480 frame, but 50 Hz PAL, you should set (vfp==52, vsync==6, vbp==87). In this case, you add 48 both front porches, right? How is that proportionate? > Those extra vbp lines will be treated as a black bar at the top of the frame, > and extra vfp lines will be at the bottom of the frame. > > However if someone specifies e.g. 720x604, there's nothing more you could > remove from vfp, so your only option is to reduce vbp compared to the standard > mode, so you'll end up with (vfp==4, vsync==6, vbp==11). The image will not be > centered, the
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Fri, 09 Sep 2022 13:39:37 + Simon Ser wrote: > On Friday, September 9th, 2022 at 12:12, Hans de Goede > wrote: > > > Phase 3: Deprecate /sys/class/backlight uAPI > > > > > > Once most userspace has moved over to using the new drm_connector > > brightness props, a Kconfig option can be added to stop exporting > > the backlight-devices under /sys/class/backlight. The plan is to > > just disable the sysfs interface and keep the existing backlight-device > > internal kernel abstraction as is, since some abstraction for (non GPU > > native) backlight devices will be necessary regardless. > > > > It is unsure if we will ever be able to do this. For example people using > > non fully integrated desktop environments like e.g. sway often use custom > > scripts binded to hotkeys to get functionality like the brightness > > up/down keyboard hotkeys changing the brightness. This typically involves > > e.g. the xbacklight utility. > > > > Even if the xbacklight utility is ported to use kms with the new connector > > object brightness properties then this still will not work because > > changing the properties will require drm-master rights and e.g. sway will > > already hold those. > > I replied to this here in another thread [1]. > > tl;dr I think it would be fine even for Sway-like compositors. Furthermore, if other compositors are like Weston in their KMS state handling, and do not track which property has already been programmed to KMS and which hasn't, and instead just smash all KMS properties every update anyway (it's also great for debugging, you always have the full state in flight), anything changed via sysfs will be immediately reverted. Therefore I think there is a high probability that the external or sysfs controls just naturally stop working anyway, even if the kernel does not remove them first. Thanks, pq > (Also note the utilities used right now are not xbacklight, but > brightnessctl/light/brillo/etc [2]) > > [1]: > https://lore.kernel.org/dri-devel/bZJU9OkYWFyaLHVa4XUE4d5iBTPFXBRyPe1wMd_ztKh5VBMu-EDNGoUDpvwtFn_u9-JMvN8QmIZVS3pzMZM_hZTkTCA9gOBnCGXc5HFmsnc=@emersion.fr/ > [2]: https://github.com/swaywm/sway/wiki#xbacklight > pgpWmDQJXU6Wl.pgp Description: OpenPGP digital signature
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Friday, September 9th, 2022 at 12:12, Hans de Goede wrote: > Phase 3: Deprecate /sys/class/backlight uAPI > > > Once most userspace has moved over to using the new drm_connector > brightness props, a Kconfig option can be added to stop exporting > the backlight-devices under /sys/class/backlight. The plan is to > just disable the sysfs interface and keep the existing backlight-device > internal kernel abstraction as is, since some abstraction for (non GPU > native) backlight devices will be necessary regardless. > > It is unsure if we will ever be able to do this. For example people using > non fully integrated desktop environments like e.g. sway often use custom > scripts binded to hotkeys to get functionality like the brightness > up/down keyboard hotkeys changing the brightness. This typically involves > e.g. the xbacklight utility. > > Even if the xbacklight utility is ported to use kms with the new connector > object brightness properties then this still will not work because > changing the properties will require drm-master rights and e.g. sway will > already hold those. I replied to this here in another thread [1]. tl;dr I think it would be fine even for Sway-like compositors. (Also note the utilities used right now are not xbacklight, but brightnessctl/light/brillo/etc [2]) [1]: https://lore.kernel.org/dri-devel/bZJU9OkYWFyaLHVa4XUE4d5iBTPFXBRyPe1wMd_ztKh5VBMu-EDNGoUDpvwtFn_u9-JMvN8QmIZVS3pzMZM_hZTkTCA9gOBnCGXc5HFmsnc=@emersion.fr/ [2]: https://github.com/swaywm/sway/wiki#xbacklight > The drm_connector brightness properties > === > > The new uAPI for this consists of 2 properties: > > 1. "display brightness": rw 0-int32_max property controlling the brightness > setting > of the connected display. The actual maximum of this will be less then > int32_max and is given in "display brightness max". > > Unlike the /sys/class/backlight/foo/brightness this brightness property > has a clear definition for the value 0. The kernel must ensure that 0 > means minimum brightness (so 0 should never turn the backlight off). > If necessary the kernel must enforce a minimum value by adding > an offset to the value seen in the property to ensure this behavior. > > For example if necessary the driver must clamp 0-255 to 10-255, which then > becomes 0-245 on the brightness property, adding 10 internally to writes > done to the brightness property. This adding of an extra offset when > necessary must only be done on the brightness property, > the /sys/class/backlight interface should be left unchanged to not break > userspace which may rely on 0 = off on some systems. > > Note amdgpu already does something like this even for /sys/class/backlight, > see the use of AMDGPU_DM_DEFAULT_MIN_BACKLIGHT in amdgpu. > > Also whenever possible the kernel must ensure that the brightness range > is in perceived brightness, but this cannot always be guaranteed. > > > 2. "display brightness max": ro 0-int32_max property giving the actual maximum > of the display's brightness setting. This will report 0 when brightness > control is not available. > > The value of "display brightness max" may change at runtime, either by > a legacy drivers/platform/x86 backlight driver loading after the drm > driver has loaded; or when plugging in a monitor which allows brightness > control over DDC/CI. In both these cases the max value will change from 0 > to the actual max value, indicating backlight control has become available > on this connector. The kernel will need to ensure that a hotplug uevent is sent to user-space at this point. Otherwise user-space has no way to figure out that the prop has changed. Overall this all looks very solid to me! Simon
Re: [PATCH] drm/rockchip: vop2: Fix eDP/HDMI sync polarities
On Mon, 15 Aug 2022 15:39:42 +0200, Sascha Hauer wrote: > The hsync/vsync polarities were not honoured for the eDP and HDMI ports. > Add the register settings to configure the polarities as requested by the > DRM_MODE_FLAG_PHSYNC/DRM_MODE_FLAG_PVSYNC flags. Applied, thanks! [1/1] drm/rockchip: vop2: Fix eDP/HDMI sync polarities commit: 35b513a74eabf09bd718e04fd9e62b09c022807f Best regards, -- Heiko Stuebner
Re: [PATCH] drm/vkms: fix 32bit compilation error by replacing macros
On Fri, Sep 9, 2022 at 7:42 AM Melissa Wen wrote:
>
> Replace vkms_formats macros for fixed-point operations with functions
> from drm/drm_fixed.h to do the same job and fix 32-bit compilation
> errors.
>
> Fixes: a19c2ac9858 ("drm: vkms: Add support to the RGB565 format")
> Tested-by: Sudip Mukherjee
> Reported-by: Sudip Mukherjee
> Reported-by: kernel test robot
> Signed-off-by: Melissa Wen
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/vkms/vkms_formats.c | 53 +++--
> 1 file changed, 19 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c
> b/drivers/gpu/drm/vkms/vkms_formats.c
> index 300abb4d1dfe..ddcd3cfeeaac 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -1,27 +1,12 @@
> // SPDX-License-Identifier: GPL-2.0+
>
> -#include
> +#include
> #include
> +#include
> +#include
>
> #include "vkms_formats.h"
>
> -/* The following macros help doing fixed point arithmetic. */
> -/*
> - * With Fixed-Point scale 15 we have 17 and 15 bits of integer and fractional
> - * parts respectively.
> - * | 0.000 |
> - * 31 0
> - */
> -#define SHIFT 15
> -
> -#define INT_TO_FIXED(a) ((a) << SHIFT)
> -#define FIXED_MUL(a, b) ((s32)(((s64)(a) * (b)) >> SHIFT))
> -#define FIXED_DIV(a, b) ((s32)(((s64)(a) << SHIFT) / (b)))
> -/* This macro converts a fixed point number to int, and round half up it */
> -#define FIXED_TO_INT_ROUND(a) (((a) + (1 << (SHIFT - 1))) >> SHIFT)
> -#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
> -#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
> -
> static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x,
> int y)
> {
> return frame_info->offset + (y * frame_info->pitch)
> @@ -137,19 +122,19 @@ static void RGB565_to_argb_u16(struct line_buffer
> *stage_buffer,
> int x_limit = min_t(size_t, drm_rect_width(_info->dst),
>stage_buffer->n_pixels);
>
> - s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
> - s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
> + s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
> + s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
>
> for (size_t x = 0; x < x_limit; x++, src_pixels++) {
> u16 rgb_565 = le16_to_cpu(*src_pixels);
> - s32 fp_r = INT_TO_FIXED((rgb_565 >> 11) & 0x1f);
> - s32 fp_g = INT_TO_FIXED((rgb_565 >> 5) & 0x3f);
> - s32 fp_b = INT_TO_FIXED(rgb_565 & 0x1f);
> + s32 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
> + s32 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
> + s32 fp_b = drm_int2fixp(rgb_565 & 0x1f);
>
> out_pixels[x].a = (u16)0x;
> - out_pixels[x].r = FIXED_TO_INT_ROUND(FIXED_MUL(fp_r,
> fp_rb_ratio));
> - out_pixels[x].g = FIXED_TO_INT_ROUND(FIXED_MUL(fp_g,
> fp_g_ratio));
> - out_pixels[x].b = FIXED_TO_INT_ROUND(FIXED_MUL(fp_b,
> fp_rb_ratio));
> + out_pixels[x].r = drm_fixp2int(drm_fixp_mul(fp_r,
> fp_rb_ratio));
> + out_pixels[x].g = drm_fixp2int(drm_fixp_mul(fp_g,
> fp_g_ratio));
> + out_pixels[x].b = drm_fixp2int(drm_fixp_mul(fp_b,
> fp_rb_ratio));
> }
> }
>
> @@ -248,17 +233,17 @@ static void argb_u16_to_RGB565(struct vkms_frame_info
> *frame_info,
> int x_limit = min_t(size_t, drm_rect_width(_info->dst),
> src_buffer->n_pixels);
>
> - s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
> - s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
> + s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
> + s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
>
> for (size_t x = 0; x < x_limit; x++, dst_pixels++) {
> - s32 fp_r = INT_TO_FIXED(in_pixels[x].r);
> - s32 fp_g = INT_TO_FIXED(in_pixels[x].g);
> - s32 fp_b = INT_TO_FIXED(in_pixels[x].b);
> + s32 fp_r = drm_int2fixp(in_pixels[x].r);
> + s32 fp_g = drm_int2fixp(in_pixels[x].g);
> + s32 fp_b = drm_int2fixp(in_pixels[x].b);
>
> - u16 r = FIXED_TO_INT_ROUND(FIXED_DIV(fp_r, fp_rb_ratio));
> - u16 g = FIXED_TO_INT_ROUND(FIXED_DIV(fp_g, fp_g_ratio));
> - u16 b = FIXED_TO_INT_ROUND(FIXED_DIV(fp_b, fp_rb_ratio));
> + u16 r = drm_fixp2int(drm_fixp_div(fp_r, fp_rb_ratio));
> + u16 g = drm_fixp2int(drm_fixp_div(fp_g, fp_g_ratio));
> + u16 b = drm_fixp2int(drm_fixp_div(fp_b, fp_rb_ratio));
>
> *dst_pixels = cpu_to_le16(r << 11 | g << 5 | b);
> }
> --
> 2.35.1
>
Re: [PATCH v2] drm: omapdrm: Improve check for contiguous buffers
On 25/08/2022 19:26, Andrew Davis wrote: While a scatter-gather table having only 1 entry does imply it is contiguous, it is a logic error to assume the inverse. Tables can have more than 1 entry and still be contiguous. Use a proper check here. Signed-off-by: Andrew Davis --- Changes from v1: - Sent correct version of patch :) Looks fine to me. But where do you need this? Are contiguous buffers with multiple sgt entries handled correctly elsewhere in the driver, i.e. do they work? =) Tomi
Re: [PATCH] drm/tidss: fix repeated words in comments
On 24/08/2022 16:04, Jilin Yuan wrote: Delete the redundant word 'to'. Signed-off-by: Jilin Yuan --- drivers/gpu/drm/tidss/tidss_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c index 666e527a0acf..7de3a5ffe5bc 100644 --- a/drivers/gpu/drm/tidss/tidss_kms.c +++ b/drivers/gpu/drm/tidss/tidss_kms.c @@ -71,7 +71,7 @@ static int tidss_atomic_check(struct drm_device *ddev, * changes. This is needed for updating the plane positions in * tidss_crtc_position_planes() which is called from * crtc_atomic_enable() and crtc_atomic_flush(). We have an -* extra flag to to mark x,y-position changes and together +* extra flag to mark x,y-position changes and together * with zpos_changed the condition recognizes all the above * cases. */ Thanks, applying to drm-misc. Tomi
Re: [PATCH] drm/omap: dss: Fix refcount leak bugs
On 22/07/2022 17:43, Liang He wrote:
In dss_init_ports() and __dss_uninit_ports(), we should call
of_node_put() for the reference returned by of_graph_get_port_by_id()
in fail path or when it is not used anymore.
Fixes: 09bffa6e5192 ("drm: omap: use common OF graph helpers")
Signed-off-by: Liang He
---
drivers/gpu/drm/omapdrm/dss/dss.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c
b/drivers/gpu/drm/omapdrm/dss/dss.c
index 0399f3390a0a..c4febb861910 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss.c
@@ -1176,6 +1176,7 @@ static void __dss_uninit_ports(struct dss_device *dss,
unsigned int num_ports)
default:
break;
}
+ of_node_put(port);
}
}
@@ -1208,11 +1209,13 @@ static int dss_init_ports(struct dss_device *dss)
default:
break;
}
+ of_node_put(port);
}
return 0;
error:
+ of_node_put(port);
__dss_uninit_ports(dss, i);
return r;
}
Thanks, applying to drm-misc.
Tomi
Re: [PATCH] drm: omapdrm: dss: replace ternary operator with max()
On 17/05/2022 08:02, Guo Zhengkui wrote: Fix the following coccicheck warning: drivers/gpu/drm/omapdrm/dss/dispc.c:2454:21-22: WARNING opportunity for max() Signed-off-by: Guo Zhengkui --- drivers/gpu/drm/omapdrm/dss/dispc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c index c4de142cc85b..0ee344ebcd1c 100644 --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -2451,7 +2451,7 @@ static int dispc_ovl_calc_scaling_44xx(struct dispc_device *dispc, *decim_x = DIV_ROUND_UP(width, in_width_max); - *decim_x = *decim_x > decim_x_min ? *decim_x : decim_x_min; + *decim_x = max(*decim_x, decim_x_min); if (*decim_x > *x_predecim) return -EINVAL; Thanks, applying to drm-misc. Tomi
Re: Meeting (BOF) at Plumbers Dublin to discuss backlight brightness as connector object property RFC?
On Fri, Sep 09, 2022 at 12:23:53PM +0200, Hans de Goede wrote: > Hi All, > > I will be at Plumbers Dublin next week and I was wondering if > anyone interested in this wants to get together for a quick > discussion / birds of a feather session about this? > > I have just posted version 2 of the RFC: > https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86...@redhat.com/T/#u > > If you are interested in meeting up please send me > an email off-list (no need to spam the list further with this) > also please let me know if there are any times which do not > work for you, and/or times which have your preference. > > I don't have a specific room/time for this yet, but if people > are interested I will try to get a room from the organization > and if that does not work out I'm sure we will figure something > out. > > One of the things which I would like to discuss is using > the backlight brightness as connector object property vs > external (not part of the compositor) tools to control the > brightness like e.g. xbacklight, quoting from the RFC: > > "people using > non fully integrated desktop environments like e.g. sway often use custom > scripts binded to hotkeys to get functionality like the brightness > up/down keyboard hotkeys changing the brightness. This typically involves > e.g. the xbacklight utility. > > Even if the xbacklight utility is ported to use kms with the new connector > object brightness properties then this still will not work because > changing the properties will require drm-master rights and e.g. sway will > already hold those." > > Note one obvious solution here would be for these use-cases to keep > using the old /sys/class/backlight interface for this, with the downside > that we will then be stuck to that interface for ever... Isn't xbacklight the thing that only works when you *have* the backlight property? Ie. currently only works on intel ddx which adds a custom property but doesn't work on modesetting ddx for example. -- Ville Syrjälä Intel
[PATCH 4/6] drm/gma500: Remove a couple of not useful function wrappers
The gma_crtc_set_config() and psb_unlocked_ioctl() functions are 1:1
wrappers for drm_helpers. Drop these wrappers.
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/gma500/gma_display.c | 8 +---
drivers/gpu/drm/gma500/gma_display.h | 2 --
drivers/gpu/drm/gma500/psb_drv.c | 8 +---
3 files changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gma_display.c
b/drivers/gpu/drm/gma500/gma_display.c
index bdbd2afa8171..fe7b8436f87a 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -555,17 +555,11 @@ int gma_crtc_page_flip(struct drm_crtc *crtc,
return ret;
}
-int gma_crtc_set_config(struct drm_mode_set *set,
- struct drm_modeset_acquire_ctx *ctx)
-{
- return drm_crtc_helper_set_config(set, ctx);
-}
-
const struct drm_crtc_funcs gma_crtc_funcs = {
.cursor_set = gma_crtc_cursor_set,
.cursor_move = gma_crtc_cursor_move,
.gamma_set = gma_crtc_gamma_set,
- .set_config = gma_crtc_set_config,
+ .set_config = drm_crtc_helper_set_config,
.destroy = gma_crtc_destroy,
.page_flip = gma_crtc_page_flip,
.enable_vblank = gma_crtc_enable_vblank,
diff --git a/drivers/gpu/drm/gma500/gma_display.h
b/drivers/gpu/drm/gma500/gma_display.h
index 113cf048105e..c8b611a2f6c6 100644
--- a/drivers/gpu/drm/gma500/gma_display.h
+++ b/drivers/gpu/drm/gma500/gma_display.h
@@ -69,8 +69,6 @@ extern int gma_crtc_page_flip(struct drm_crtc *crtc,
struct drm_pending_vblank_event *event,
uint32_t page_flip_flags,
struct drm_modeset_acquire_ctx *ctx);
-extern int gma_crtc_set_config(struct drm_mode_set *set,
- struct drm_modeset_acquire_ctx *ctx);
extern void gma_crtc_save(struct drm_crtc *crtc);
extern void gma_crtc_restore(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 7a94e0d8fa6c..8266371aeac1 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -430,12 +430,6 @@ static inline void get_brightness(struct backlight_device
*bd)
#endif
}
-static long psb_unlocked_ioctl(struct file *filp, unsigned int cmd,
- unsigned long arg)
-{
- return drm_ioctl(filp, cmd, arg);
-}
-
static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
struct drm_psb_private *dev_priv;
@@ -497,7 +491,7 @@ static const struct file_operations psb_gem_fops = {
.owner = THIS_MODULE,
.open = drm_open,
.release = drm_release,
- .unlocked_ioctl = psb_unlocked_ioctl,
+ .unlocked_ioctl = drm_ioctl,
.compat_ioctl = drm_compat_ioctl,
.mmap = drm_gem_mmap,
.poll = drm_poll,
--
2.37.2
[PATCH 5/6] drm/gma500: Rewrite power management code
Rewrite the power.c code. For some reason this was doing locking +
refcounting + state (suspended or not) bookkeeping all by itself.
But there is no reason for this, this is all taken care of by
the runtime-pm core, through pm_runtime_get()/_put().
Besides this not being necessary the DIY code is also quite weird/
buggy in some places. E.g. power_begin() would manually do a resume
when not resumed already and force_on=true, followed by a
pm_runtime_get(), which will cause a call to gma_power_resume() to
get scheduled which would redo the entire resume again. Which can
all be replaced by a single pm_runtime_get_sync() call.
Note that this is just a cleanup, this does not actually fix
the (disabled through #if 0) runtime-pm support. It does now call
pm_runtime_enable(), but only after doing a pm_runtime_get() at
probe-time, so the device is never runtime suspended.
Doing this permanent get() + enable() instead of not calling
enable() at all is necessary for the pm_runtime_get_if_in_use() call
in gma_power_begin() to work properly.
Note this also removes the gma_power_is_on() call a check like this
without actually holding a reference is always racy, so it is a bad
idea (and therefor has no pm_runtime_foo() equivalent).
The 2 code paths which were using gma_power_is_on() are actually both
guaranteed to only run when the device is powered-on so the 2 checks
can simply be dropped.
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/gma500/oaktrail_lvds.c | 1 -
drivers/gpu/drm/gma500/power.c | 131 ++---
drivers/gpu/drm/gma500/power.h | 9 --
drivers/gpu/drm/gma500/psb_drv.c | 6 --
drivers/gpu/drm/gma500/psb_drv.h | 4 +-
drivers/gpu/drm/gma500/psb_irq.c | 15 ++-
6 files changed, 37 insertions(+), 129 deletions(-)
diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c
b/drivers/gpu/drm/gma500/oaktrail_lvds.c
index 4d98df189e10..75b4eb1c8884 100644
--- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
+++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
@@ -61,7 +61,6 @@ static void oaktrail_lvds_set_power(struct drm_device *dev,
pp_status = REG_READ(PP_STATUS);
} while (pp_status & PP_ON);
dev_priv->is_lvds_on = false;
- pm_request_idle(dev->dev);
}
gma_power_end(dev);
}
diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
index 66873085d450..5dd291dad4a4 100644
--- a/drivers/gpu/drm/gma500/power.c
+++ b/drivers/gpu/drm/gma500/power.c
@@ -37,9 +37,6 @@
#include
#include
-static struct mutex power_mutex; /* Serialize power ops */
-static DEFINE_SPINLOCK(power_ctrl_lock); /* Serialize power claim */
-
/**
* gma_power_init - initialise power manager
* @dev: our device
@@ -54,13 +51,22 @@ void gma_power_init(struct drm_device *dev)
dev_priv->apm_base = dev_priv->apm_reg & 0x;
dev_priv->ospm_base &= 0x;
- dev_priv->display_power = true; /* We start active */
- dev_priv->display_count = 0;/* Currently no users */
- dev_priv->suspended = false;/* And not suspended */
- mutex_init(_mutex);
-
if (dev_priv->ops->init_pm)
dev_priv->ops->init_pm(dev);
+
+ /*
+* Runtime pm support is broken atm. So for now unconditionally
+* call pm_runtime_get() here and put it again in psb_driver_unload()
+*
+* To fix this we need to call pm_runtime_get() once for each active
+* pipe at boot and then put() / get() for each pipe disable / enable
+* so that the device gets runtime suspended when no pipes are active.
+*/
+ pm_runtime_get(dev->dev);
+ pm_runtime_set_active(dev->dev); /* Must be done before
pm_runtime_enable()! */
+ pm_runtime_enable(dev->dev);
+
+ dev_priv->pm_initialized = true;
}
/**
@@ -71,8 +77,13 @@ void gma_power_init(struct drm_device *dev)
*/
void gma_power_uninit(struct drm_device *dev)
{
+ struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+
+ if (!dev_priv->pm_initialized)
+ return;
+
pm_runtime_disable(dev->dev);
- pm_runtime_set_suspended(dev->dev);
+ pm_runtime_put_noidle(dev->dev);
}
/**
@@ -85,11 +96,8 @@ static void gma_suspend_display(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
- if (dev_priv->suspended)
- return;
dev_priv->ops->save_regs(dev);
dev_priv->ops->power_down(dev);
- dev_priv->display_power = false;
}
/**
@@ -106,8 +114,6 @@ static void gma_resume_display(struct pci_dev *pdev)
/* turn on the display power island */
dev_priv->ops->power_up(dev);
- dev_priv->suspended = false;
- dev_priv->display_power = true;
PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
pci_write_config_word(pdev,
[PATCH 6/6] drm/gma500: Remove unnecessary suspend/resume wrappers
The psb_runtime_suspend/resume/thaw/freeze/restore functions are all
just 1:1 wrappers around gma_power_suspend/_resume.
Drop these wrappers and use the DEFINE_RUNTIME_DEV_PM_OPS() macro to
define the dev_pm_ops struct.
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/gma500/power.c | 25 -
drivers/gpu/drm/gma500/power.h | 9 -
drivers/gpu/drm/gma500/psb_drv.c | 10 +-
3 files changed, 1 insertion(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
index 5dd291dad4a4..62d2cc1923f1 100644
--- a/drivers/gpu/drm/gma500/power.c
+++ b/drivers/gpu/drm/gma500/power.c
@@ -230,28 +230,3 @@ void gma_power_end(struct drm_device *dev)
{
pm_runtime_put(dev->dev);
}
-
-int psb_runtime_suspend(struct device *dev)
-{
- return gma_power_suspend(dev);
-}
-
-int psb_runtime_resume(struct device *dev)
-{
- return gma_power_resume(dev);
-}
-
-int gma_power_thaw(struct device *_dev)
-{
- return gma_power_resume(_dev);
-}
-
-int gma_power_freeze(struct device *_dev)
-{
- return gma_power_suspend(_dev);
-}
-
-int gma_power_restore(struct device *_dev)
-{
- return gma_power_resume(_dev);
-}
diff --git a/drivers/gpu/drm/gma500/power.h b/drivers/gpu/drm/gma500/power.h
index 243b9dd00910..063328d66652 100644
--- a/drivers/gpu/drm/gma500/power.h
+++ b/drivers/gpu/drm/gma500/power.h
@@ -43,9 +43,6 @@ void gma_power_uninit(struct drm_device *dev);
*/
int gma_power_suspend(struct device *dev);
int gma_power_resume(struct device *dev);
-int gma_power_thaw(struct device *dev);
-int gma_power_freeze(struct device *dev);
-int gma_power_restore(struct device *_dev);
/*
* These are the functions the driver should use to wrap all hw access
@@ -54,10 +51,4 @@ int gma_power_restore(struct device *_dev);
bool gma_power_begin(struct drm_device *dev, bool force);
void gma_power_end(struct drm_device *dev);
-/*
- * GFX-Runtime PM callbacks
- */
-int psb_runtime_suspend(struct device *dev);
-int psb_runtime_resume(struct device *dev);
-
#endif /*_PSB_POWERMGMT_H_*/
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index aea8876059d8..9168bc620628 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -471,15 +471,7 @@ static void psb_pci_remove(struct pci_dev *pdev)
drm_dev_unregister(dev);
}
-static const struct dev_pm_ops psb_pm_ops = {
- .resume = gma_power_resume,
- .suspend = gma_power_suspend,
- .thaw = gma_power_thaw,
- .freeze = gma_power_freeze,
- .restore = gma_power_restore,
- .runtime_suspend = psb_runtime_suspend,
- .runtime_resume = psb_runtime_resume,
-};
+static DEFINE_RUNTIME_DEV_PM_OPS(psb_pm_ops, gma_power_suspend,
gma_power_resume, NULL);
static const struct file_operations psb_gem_fops = {
.owner = THIS_MODULE,
--
2.37.2
[PATCH 2/6] drm/gma500: Remove runtime_allowed dead code in psb_unlocked_ioctl()
runtime_allowed is initialized to 0, so the runtime_allowed == 1 condition
is never true making this dead code. Remove it.
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/gma500/psb_drv.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 54e756b48606..7a94e0d8fa6c 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -433,18 +433,7 @@ static inline void get_brightness(struct backlight_device
*bd)
static long psb_unlocked_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
- struct drm_file *file_priv = filp->private_data;
- struct drm_device *dev = file_priv->minor->dev;
- struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
- static unsigned int runtime_allowed;
-
- if (runtime_allowed == 1 && dev_priv->is_lvds_on) {
- runtime_allowed++;
- pm_runtime_allow(dev->dev);
- dev_priv->rpm_enabled = 1;
- }
return drm_ioctl(filp, cmd, arg);
- /* FIXME: do we need to wrap the other side of this */
}
static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
--
2.37.2
[PATCH 0/6] drm/gma500: 1 fix + further cleanups
Hi Patrik, Here is another gma500 patch-series with one more bugfix and a bunch of other cleanups of stuff which I noticed while doing the previous set of bugfixes. Regards, Hans Hans de Goede (6): drm/gma500: Wait longer for the GPU to power-down drm/gma500: Remove runtime_allowed dead code in psb_unlocked_ioctl() drm/gma500: Remove never set dev_priv->rpm_enabled flag drm/gma500: Remove a couple of not useful function wrappers drm/gma500: Rewrite power management code drm/gma500: Remove unnecessary suspend/resume wrappers drivers/gpu/drm/gma500/cdv_device.c| 2 +- drivers/gpu/drm/gma500/gma_display.c | 19 +-- drivers/gpu/drm/gma500/gma_display.h | 2 - drivers/gpu/drm/gma500/oaktrail_lvds.c | 1 - drivers/gpu/drm/gma500/power.c | 156 + drivers/gpu/drm/gma500/power.h | 18 --- drivers/gpu/drm/gma500/psb_drv.c | 35 +- drivers/gpu/drm/gma500/psb_drv.h | 7 +- drivers/gpu/drm/gma500/psb_irq.c | 15 ++- 9 files changed, 41 insertions(+), 214 deletions(-) -- 2.37.2
[PATCH 3/6] drm/gma500: Remove never set dev_priv->rpm_enabled flag
The rpm_enabled flag is never set, remove it.
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/gma500/gma_display.c | 13 +
drivers/gpu/drm/gma500/psb_drv.h | 3 ---
2 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gma_display.c
b/drivers/gpu/drm/gma500/gma_display.c
index 2f52eceda3a1..bdbd2afa8171 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -558,18 +558,7 @@ int gma_crtc_page_flip(struct drm_crtc *crtc,
int gma_crtc_set_config(struct drm_mode_set *set,
struct drm_modeset_acquire_ctx *ctx)
{
- struct drm_device *dev = set->crtc->dev;
- struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
- int ret;
-
- if (!dev_priv->rpm_enabled)
- return drm_crtc_helper_set_config(set, ctx);
-
- pm_runtime_forbid(dev->dev);
- ret = drm_crtc_helper_set_config(set, ctx);
- pm_runtime_allow(dev->dev);
-
- return ret;
+ return drm_crtc_helper_set_config(set, ctx);
}
const struct drm_crtc_funcs gma_crtc_funcs = {
diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
index 731cc356c07a..dd6fd49d85f3 100644
--- a/drivers/gpu/drm/gma500/psb_drv.h
+++ b/drivers/gpu/drm/gma500/psb_drv.h
@@ -486,9 +486,6 @@ struct drm_psb_private {
unsigned int core_freq;
uint32_t iLVDS_enable;
- /* Runtime PM state */
- int rpm_enabled;
-
/* MID specific */
bool use_msi;
bool has_gct;
--
2.37.2
[PATCH 1/6] drm/gma500: Wait longer for the GPU to power-down
On a cedartrail during boot the following error was logged each boot: [ 12.168341] gma500 :00:02.0: GPU: power management timed out. Increase the timeout to fix this. Sometimes the display also failed to come up properly (userspace was running normally, but the LCD was showing all black as contents). Hopefully this also fix this issue. Signed-off-by: Hans de Goede --- drivers/gpu/drm/gma500/cdv_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c index ce96234f3df2..ff5104fe5692 100644 --- a/drivers/gpu/drm/gma500/cdv_device.c +++ b/drivers/gpu/drm/gma500/cdv_device.c @@ -230,7 +230,7 @@ static void cdv_init_pm(struct drm_device *dev) u32 pwr_sts = inl(dev_priv->apm_base + PSB_APM_STS); if ((pwr_sts & PSB_PWRGT_GFX_MASK) == 0) return; - udelay(10); + usleep_range(100, 1000); } dev_err(dev->dev, "GPU: power management timed out.\n"); } -- 2.37.2
Re: Meeting (BOF) at Plumbers Dublin to discuss backlight brightness as connector object property RFC?
On Friday, September 9th, 2022 at 12:23, Hans de Goede wrote: > "people using > non fully integrated desktop environments like e.g. sway often use custom > scripts binded to hotkeys to get functionality like the brightness > up/down keyboard hotkeys changing the brightness. This typically involves > e.g. the xbacklight utility. > > Even if the xbacklight utility is ported to use kms with the new connector > object brightness properties then this still will not work because > changing the properties will require drm-master rights and e.g. sway will > already hold those." I don't think this is a good argument. Sway (which I'm a maintainer of) can add a command to change the backlight, and then users can bind their keybinding to that command. This is not very different from e.g. a keybind to switch on/off a monitor. We can also standardize a protocol to change the backlight across all non-fully-integrated desktop environments (would be a simple addition to output-power-management [1]), so that a single tool can work for multiple compositors. Simon [1]: https://gitlab.freedesktop.org/wlroots/wlr-protocols/-/issues/114
