[PATCH 0/4] drm/i915: Fix older platforms
From: Ville Syrjälä Fix a div-by-zero on gen2, and make the L-shaped memory detection actually work on cl/ctg. Atm the SWIZZLE_UNKNOWN stuff just trips some GEM_BUG_ONs. This doesn't fix those but since I populate all my memory channels symmetrically I get to avoid the GEM_BUG_ONs by correctly detecting that I don't have an L-shaped memory configuration. Ville Syrjälä (4): drm/i915: Avoid div-by-zero on gen2 drm/i915: Read C0DRB3/C1DRB3 as 16 bits again drm/i915: Give C0DRB3/C1DRB3 a _BW suffix drm/i915: Rewrite CL/CTG L-shaped memory detection drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 19 ++- drivers/gpu/drm/i915/i915_debugfs.c | 16 drivers/gpu/drm/i915/i915_reg.h | 8 ++-- 4 files changed, 29 insertions(+), 16 deletions(-) -- 2.26.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/i915: Avoid div-by-zero on gen2
From: Ville Syrjälä Gen2 tiles are 2KiB in size so i915_gem_object_get_tile_row_size() can in fact return <4KiB, which leads to div-by-zero here. Avoid that. Not sure i915_gem_object_get_tile_row_size() is entirely sane anyway since it doesn't account for the different tile layouts on i8xx/i915... I'm not able to hit this before commit 6846895fde05 ("drm/i915: Replace PIN_NONFAULT with calls to PIN_NOEVICT") and it looks like I also need to run recent version of Mesa. With those in place xonotic trips on this quite easily on my 85x. Cc: sta...@vger.kernel.org Reviewed-by: Chris Wilson Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index 2561a2f1e54f..8598a1c78a4c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -189,7 +189,7 @@ compute_partial_view(const struct drm_i915_gem_object *obj, struct i915_ggtt_view view; if (i915_gem_object_is_tiled(obj)) - chunk = roundup(chunk, tile_row_pages(obj)); + chunk = roundup(chunk, tile_row_pages(obj) ?: 1); view.type = I915_GGTT_VIEW_PARTIAL; view.partial.offset = rounddown(page_offset, chunk); -- 2.26.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/i915: Read C0DRB3/C1DRB3 as 16 bits again
From: Ville Syrjälä We've defined C0DRB3/C0DRB3 as 16 bit registers, so access them as such. Fixes: 1c8242c3a4b2 ("drm/i915: Use unchecked writes for setting up the fences") Reviewed-by: Chris Wilson Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c index e72b7a0dc316..8a322594210c 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c @@ -653,8 +653,8 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt) * banks of memory are paired and unswizzled on the * uneven portion, so leave that as unknown. */ - if (intel_uncore_read(uncore, C0DRB3) == - intel_uncore_read(uncore, C1DRB3)) { + if (intel_uncore_read16(uncore, C0DRB3) == + intel_uncore_read16(uncore, C1DRB3)) { swizzle_x = I915_BIT_6_SWIZZLE_9_10; swizzle_y = I915_BIT_6_SWIZZLE_9; } -- 2.26.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/i915: Give C0DRB3/C1DRB3 a _BW suffix
From: Ville Syrjälä These are the 965g/g45/g33 specific DRB registers. Give them a suitable suffix so we can add their counterparts for other platforms. Reviewed-by: Chris Wilson Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++-- drivers/gpu/drm/i915/i915_debugfs.c | 4 ++-- drivers/gpu/drm/i915/i915_reg.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c index 8a322594210c..0fa6c38893f7 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c @@ -653,8 +653,8 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt) * banks of memory are paired and unswizzled on the * uneven portion, so leave that as unknown. */ - if (intel_uncore_read16(uncore, C0DRB3) == - intel_uncore_read16(uncore, C1DRB3)) { + if (intel_uncore_read16(uncore, C0DRB3_BW) == + intel_uncore_read16(uncore, C1DRB3_BW)) { swizzle_x = I915_BIT_6_SWIZZLE_9_10; swizzle_y = I915_BIT_6_SWIZZLE_9; } diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index b654b7498bcd..8dd374691102 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -622,9 +622,9 @@ static int i915_swizzle_info(struct seq_file *m, void *data) seq_printf(m, "DDC2 = 0x%08x\n", intel_uncore_read(uncore, DCC2)); seq_printf(m, "C0DRB3 = 0x%04x\n", - intel_uncore_read16(uncore, C0DRB3)); + intel_uncore_read16(uncore, C0DRB3_BW)); seq_printf(m, "C1DRB3 = 0x%04x\n", - intel_uncore_read16(uncore, C1DRB3)); + intel_uncore_read16(uncore, C1DRB3_BW)); } else if (INTEL_GEN(dev_priv) >= 6) { seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n", intel_uncore_read(uncore, MAD_DIMM_C0)); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 66a902b3bb8e..0587b2455ea1 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3787,8 +3787,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define CSHRDDR3CTL_DDR3 (1 << 2) /* 965 MCH register controlling DRAM channel configuration */ -#define C0DRB3 _MMIO(MCHBAR_MIRROR_BASE + 0x206) -#define C1DRB3 _MMIO(MCHBAR_MIRROR_BASE + 0x606) +#define C0DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x206) +#define C1DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x606) /* snb MCH registers for reading the DRAM channel configuration */ #define MAD_DIMM_C0_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5004) -- 2.26.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/i915: Rewrite CL/CTG L-shaped memory detection
From: Ville Syrjälä Currently we try to detect a symmetric memory configurations using a magic DCC2_MODIFIED_ENHANCED_DISABLE bit. That bit is either only set on a very specific subset of machines or it just does not exist (it's not mentioned in any public chipset datasheets I've found). As it happens my CL/CTG machines never set said bit, even if I populate the channels with identical sticks. So let's do the L-shaped memory detection the same way as the desktop variants, ie. just look at the DRAM rank boundary registers to see if both channels have an identical size. With this my CL/CTG no longer claim L-shaped memory when I use identical sticks. Also tested with non-matching sticks just to make sure the L-shaped memory is still properly detected. And for completeness let's update the debugfs code to dump the correct set of registers on each platform. Cc: Chris Wilson Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 15 --- drivers/gpu/drm/i915/i915_debugfs.c | 16 drivers/gpu/drm/i915/i915_reg.h | 4 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c index 0fa6c38893f7..754f20768de5 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c @@ -693,14 +693,15 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt) swizzle_x = I915_BIT_6_SWIZZLE_9_10_17; swizzle_y = I915_BIT_6_SWIZZLE_9_17; } - break; - } - /* check for L-shaped memory aka modified enhanced addressing */ - if (IS_GEN(i915, 4) && - !(intel_uncore_read(uncore, DCC2) & DCC2_MODIFIED_ENHANCED_DISABLE)) { - swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; - swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; + /* check for L-shaped memory aka modified enhanced addressing */ + if (IS_GEN(i915, 4) && + intel_uncore_read16(uncore, C0DRB3_CL) != + intel_uncore_read16(uncore, C1DRB3_CL)) { + swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; + swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; + } + break; } if (dcc == 0x) { diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 8dd374691102..6de11ffcde38 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -621,10 +621,18 @@ static int i915_swizzle_info(struct seq_file *m, void *data) intel_uncore_read(uncore, DCC)); seq_printf(m, "DDC2 = 0x%08x\n", intel_uncore_read(uncore, DCC2)); - seq_printf(m, "C0DRB3 = 0x%04x\n", - intel_uncore_read16(uncore, C0DRB3_BW)); - seq_printf(m, "C1DRB3 = 0x%04x\n", - intel_uncore_read16(uncore, C1DRB3_BW)); + + if (IS_G45(dev_priv) || IS_I965G(dev_priv) || IS_G33(dev_priv)) { + seq_printf(m, "C0DRB3 = 0x%04x\n", + intel_uncore_read16(uncore, C0DRB3_BW)); + seq_printf(m, "C1DRB3 = 0x%04x\n", + intel_uncore_read16(uncore, C1DRB3_BW)); + } else if (IS_GEN(dev_priv, 4)) { + seq_printf(m, "C0DRB3 = 0x%04x\n", + intel_uncore_read16(uncore, C0DRB3_CL)); + seq_printf(m, "C1DRB3 = 0x%04x\n", + intel_uncore_read16(uncore, C1DRB3_CL)); + } } else if (INTEL_GEN(dev_priv) >= 6) { seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n", intel_uncore_read(uncore, MAD_DIMM_C0)); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 0587b2455ea1..055c258179a1 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3790,6 +3790,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define C0DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x206) #define C1DRB3_BW _MMIO(MCHBAR_MIRROR_BASE + 0x606) +/* 965gm,ctg DRAM channel configuration */ +#define C0DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1206) +#define C1DRB3_CL _MMIO(MCHBAR_MIRROR_BASE + 0x1306) + /* snb MCH registers for reading the DRAM channel configuration */ #define MAD_DIMM_C0_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5004) #define MAD_DIMM_C1_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5008) -- 2.26.3 ___
[PATCH] drm/vblank: Avoid storing a timestamp for the same frame twice
From: Ville Syrjälä drm_vblank_restore() exists because certain power saving states can clobber the hardware frame counter. The way it does this is by guesstimating how many frames were missed purely based on the difference between the last stored timestamp vs. a newly sampled timestamp. If we should call this function before a full frame has elapsed since we sampled the last timestamp we would end up with a possibly slightly different timestamp value for the same frame. Currently we will happily overwrite the already stored timestamp for the frame with the new value. This could cause userspace to observe two different timestamps for the same frame (and the timestamp could even go backwards depending on how much error we introduce when correcting the timestamp based on the scanout position). To avoid that let's not update the stored timestamp unless we're also incrementing the sequence counter. We do still want to update vblank->last with the freshly sampled hw frame counter value so that subsequent vblank irqs/queries can actually use the hw frame counter to determine how many frames have elapsed. Cc: Dhinakaran Pandiyan Cc: Rodrigo Vivi Cc: Daniel Vetter Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_vblank.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 893165eeddf3..e127a7db2088 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -176,6 +176,17 @@ static void store_vblank(struct drm_device *dev, unsigned int pipe, vblank->last = last; + /* +* drm_vblank_restore() wants to always update +* vblank->last since we can't trust the frame counter +* across power saving states. But we don't want to alter +* the stored timestamp for the same frame number since +* that would cause userspace to potentially observe two +* different timestamps for the same frame. +*/ + if (vblank_count_inc == 0) + return; + write_seqlock(&vblank->seqlock); vblank->time = t_vblank; atomic64_add(vblank_count_inc, &vblank->count); -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/vblank: Do not store a new vblank timestamp in drm_vblank_restore()
From: Ville Syrjälä drm_vblank_restore() exists because certain power saving states can clobber the hardware frame counter. The way it does this is by guesstimating how many frames were missed purely based on the difference between the last stored timestamp vs. a newly sampled timestamp. If we should call this function before a full frame has elapsed since we sampled the last timestamp we would end up with a possibly slightly different timestamp value for the same frame. Currently we will happily overwrite the already stored timestamp for the frame with the new value. This could cause userspace to observe two different timestamps for the same frame (and the timestamp could even go backwards depending on how much error we introduce when correcting the timestamp based on the scanout position). To avoid that let's not update the stored timestamp at all, and instead we just fix up the last recorded hw vblank counter value such that the already stored timestamp/seq number will match. Thus the next time a vblank irq happens it will calculate the correct diff between the current and stored hw vblank counter values. Sidenote: Another possible idea that came to mind would be to do this correction only if the power really was removed since the last time we sampled the hw frame counter. But to do that we would need a robust way to detect when it has occurred. Some possibilities could involve some kind of hardare power well transition counter, or potentially we could store a magic value in a scratch register that lives in the same power well. But I'm not sure either of those exist, so would need an actual investigation to find out. All of that is very hardware specific of course, so would have to be done in the driver code. Cc: Dhinakaran Pandiyan Cc: Rodrigo Vivi Cc: Daniel Vetter Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_vblank.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 2bd989688eae..3417e1ac7918 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -1478,6 +1478,7 @@ static void drm_vblank_restore(struct drm_device *dev, unsigned int pipe) u64 diff_ns; u32 cur_vblank, diff = 1; int count = DRM_TIMESTAMP_MAXRETRIES; + u32 max_vblank_count = drm_max_vblank_count(dev, pipe); if (drm_WARN_ON(dev, pipe >= dev->num_crtcs)) return; @@ -1504,7 +1505,7 @@ static void drm_vblank_restore(struct drm_device *dev, unsigned int pipe) drm_dbg_vbl(dev, "missed %d vblanks in %lld ns, frame duration=%d ns, hw_diff=%d\n", diff, diff_ns, framedur_ns, cur_vblank - vblank->last); - store_vblank(dev, pipe, diff, t_vblank, cur_vblank); + vblank->last = (cur_vblank - diff) & max_vblank_count; } /** -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Use drm_mode_is_420_only() instead of open coding it
From: Ville Syrjälä Replace the open coded drm_mode_is_420_only() with the real thing. No functional changes. Cc: Werner Sembach Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modes.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 33a93fa24eb1..12fcbb7ce179 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1176,16 +1176,11 @@ enum drm_mode_status drm_mode_validate_ycbcr420(const struct drm_display_mode *mode, struct drm_connector *connector) { - u8 vic = drm_match_cea_mode(mode); - enum drm_mode_status status = MODE_OK; - struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; - - if (test_bit(vic, hdmi->y420_vdb_modes)) { - if (!connector->ycbcr_420_allowed) - status = MODE_NO_420; - } + if (!connector->ycbcr_420_allowed && + drm_mode_is_420_only(&connector->display_info, mode)) + return MODE_NO_420; - return status; + return MODE_OK; } EXPORT_SYMBOL(drm_mode_validate_ycbcr420); -- 2.26.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/4] drm: Make modeset locking easier
From: Ville Syrjälä While staring at some DRM_MODESET_LOCK_ALL_{BEGIN,END}() conversions I decided I don't really like what those macros do. The main problems that I see: - the magic goto inside limits their usefulness to baically a single statement, unless you're willing to look inside and find out the name of the magic label - can't help at all in the "we don't want to lock everything" cases, which are quite numerous - not at all obvious that there's a loop in there So I figured I'd try to come up with something more universally useful. Cc: Sean Paul Cc: Daniel Vetter Ville Syrjälä (4): drm: Introduce drm_modeset_lock_ctx_retry() drm: Introduce drm_modeset_lock_all_ctx_retry() drm/i915: Extract intel_crtc_initial_commit() drm/i915: Use drm_modeset_lock_ctx_retry() & co. drivers/gpu/drm/drm_modeset_lock.c| 44 drivers/gpu/drm/i915/display/g4x_dp.c | 17 +- drivers/gpu/drm/i915/display/intel_audio.c| 17 +- drivers/gpu/drm/i915/display/intel_ddi.c | 16 +- drivers/gpu/drm/i915/display/intel_display.c | 192 -- .../drm/i915/display/intel_display_debugfs.c | 45 ++-- drivers/gpu/drm/i915/display/intel_pipe_crc.c | 38 ++-- include/drm/drm_modeset_lock.h| 26 +++ 8 files changed, 188 insertions(+), 207 deletions(-) -- 2.31.1
[PATCH 1/4] drm: Introduce drm_modeset_lock_ctx_retry()
From: Ville Syrjälä Quite a few places are hand rolling the modeset lock backoff dance. Let's suck that into a helper macro that is easier to use without forgetting some steps. The main downside is probably that the implementation of drm_with_modeset_lock_ctx() is a bit harder to read than a hand rolled version on account of being split across three functions, but the actual code using it ends up being much simpler. Cc: Sean Paul Cc: Daniel Vetter Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modeset_lock.c | 44 ++ include/drm/drm_modeset_lock.h | 20 ++ 2 files changed, 64 insertions(+) diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index fcfe1a03c4a1..083df96632e8 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -425,3 +425,47 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev, return 0; } EXPORT_SYMBOL(drm_modeset_lock_all_ctx); + +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx, +struct drm_atomic_state *state, +unsigned int flags, int *ret) +{ + drm_modeset_acquire_init(ctx, flags); + + if (state) + state->acquire_ctx = ctx; + + *ret = -EDEADLK; +} +EXPORT_SYMBOL(_drm_modeset_lock_begin); + +bool _drm_modeset_lock_loop(int *ret) +{ + if (*ret == -EDEADLK) { + *ret = 0; + return true; + } + + return false; +} +EXPORT_SYMBOL(_drm_modeset_lock_loop); + +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx, + struct drm_atomic_state *state, + int *ret) +{ + if (*ret == -EDEADLK) { + if (state) + drm_atomic_state_clear(state); + + *ret = drm_modeset_backoff(ctx); + if (*ret == 0) { + *ret = -EDEADLK; + return; + } + } + + drm_modeset_drop_locks(ctx); + drm_modeset_acquire_fini(ctx); +} +EXPORT_SYMBOL(_drm_modeset_lock_end); diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index aafd07388eb7..5eaad2533de5 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -26,6 +26,7 @@ #include +struct drm_atomic_state; struct drm_modeset_lock; /** @@ -203,4 +204,23 @@ modeset_lock_fail: \ if (!drm_drv_uses_atomic_modeset(dev)) \ mutex_unlock(&dev->mode_config.mutex); +void _drm_modeset_lock_begin(struct drm_modeset_acquire_ctx *ctx, +struct drm_atomic_state *state, +unsigned int flags, +int *ret); +bool _drm_modeset_lock_loop(int *ret); +void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx, + struct drm_atomic_state *state, + int *ret); + +/* + * Note that one must always use "continue" rather than + * "break" or "return" to handle errors within the + * drm_modeset_lock_ctx_retry() block. + */ +#define drm_modeset_lock_ctx_retry(ctx, state, flags, ret) \ + for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \ +_drm_modeset_lock_loop(&(ret)); \ +_drm_modeset_lock_end((ctx), (state), &(ret))) + #endif /* DRM_MODESET_LOCK_H_ */ -- 2.31.1
[PATCH 2/4] drm: Introduce drm_modeset_lock_all_ctx_retry()
From: Ville Syrjälä Layer drm_modeset_lock_all_ctx_retry() on top of drm_modeset_lock_ctx_retry() to make the fairly common "let's lock everything" pattern nicer. Currently we have DRM_MODESET_LOCK_ALL_{BEGIN,END}() for this but I don't really like it due to the magic gotos within, which makes it hard to use if you want to do multiple steps between the BEGING/END. One would either have to know the name of the magic label, or always wrap the whole thing into a function so only the single call appears between the BEGIN/END. Cc: Sean Paul Cc: Daniel Vetter Signed-off-by: Ville Syrjälä --- include/drm/drm_modeset_lock.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index 5eaad2533de5..2e2548680aaa 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -223,4 +223,10 @@ void _drm_modeset_lock_end(struct drm_modeset_acquire_ctx *ctx, _drm_modeset_lock_loop(&(ret)); \ _drm_modeset_lock_end((ctx), (state), &(ret))) +#define drm_modeset_lock_all_ctx_retry(dev, ctx, state, flags, ret) \ + for (_drm_modeset_lock_begin((ctx), (state), (flags), &(ret)); \ +_drm_modeset_lock_loop(&(ret)); \ +_drm_modeset_lock_end((ctx), (state), &(ret))) \ + for_each_if(((ret) = drm_modeset_lock_all_ctx((dev), (ctx))) == 0) + #endif /* DRM_MODESET_LOCK_H_ */ -- 2.31.1
[PATCH 3/4] drm/i915: Extract intel_crtc_initial_commit()
From: Ville Syrjälä Extract intel_crtc_initial_commit() from intel_initial_commit(). Should make subsequent changes a bit less convoluted. Cc: Sean Paul Cc: Daniel Vetter Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 96 +++- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 65ddb6ca16e6..3718399c4c2f 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -12129,12 +12129,60 @@ static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv) drm_dbg(&dev_priv->drm, "FDI PLL freq=%d\n", dev_priv->fdi_pll_freq); } +static int intel_crtc_initial_commit(struct intel_atomic_state *state, +struct intel_crtc *crtc) +{ + struct intel_crtc_state *crtc_state; + struct intel_encoder *encoder; + int ret; + + crtc_state = intel_atomic_get_crtc_state(&state->base, crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + + if (!crtc_state->hw.active) + return 0; + + /* +* We've not yet detected sink capabilities +* (audio,infoframes,etc.) and thus we don't want to +* force a full state recomputation yet. We want that to +* happen only for the first real commit from userspace. +* So preserve the inherited flag for the time being. +*/ + crtc_state->inherited = true; + + ret = drm_atomic_add_affected_planes(&state->base, &crtc->base); + if (ret) + return ret; + + /* +* FIXME hack to force a LUT update to avoid the +* plane update forcing the pipe gamma on without +* having a proper LUT loaded. Remove once we +* have readout for pipe gamma enable. +*/ + crtc_state->uapi.color_mgmt_changed = true; + + for_each_intel_encoder_mask(state->base.dev, encoder, crtc_state->uapi.encoder_mask) { + if (encoder->initial_fastset_check && + !encoder->initial_fastset_check(encoder, crtc_state)) { + ret = drm_atomic_add_affected_connectors(&state->base, +&crtc->base); + if (ret) + return ret; + } + } + + return 0; +} + static int intel_initial_commit(struct drm_device *dev) { - struct drm_atomic_state *state = NULL; struct drm_modeset_acquire_ctx ctx; + struct drm_atomic_state *state; struct intel_crtc *crtc; - int ret = 0; + int ret; state = drm_atomic_state_alloc(dev); if (!state) @@ -12146,49 +12194,9 @@ static int intel_initial_commit(struct drm_device *dev) state->acquire_ctx = &ctx; for_each_intel_crtc(dev, crtc) { - struct intel_crtc_state *crtc_state = - intel_atomic_get_crtc_state(state, crtc); - - if (IS_ERR(crtc_state)) { - ret = PTR_ERR(crtc_state); + ret = intel_crtc_initial_commit(to_intel_atomic_state(state), crtc); + if (ret) goto out; - } - - if (crtc_state->hw.active) { - struct intel_encoder *encoder; - - /* -* We've not yet detected sink capabilities -* (audio,infoframes,etc.) and thus we don't want to -* force a full state recomputation yet. We want that to -* happen only for the first real commit from userspace. -* So preserve the inherited flag for the time being. -*/ - crtc_state->inherited = true; - - ret = drm_atomic_add_affected_planes(state, &crtc->base); - if (ret) - goto out; - - /* -* FIXME hack to force a LUT update to avoid the -* plane update forcing the pipe gamma on without -* having a proper LUT loaded. Remove once we -* have readout for pipe gamma enable. -*/ - crtc_state->uapi.color_mgmt_changed = true; - - for_each_intel_encoder_mask(dev, encoder, - crtc_state->uapi.encoder_mask) { - if (encoder->initial_fastset_check && - !encoder->initial_fastset_check(encoder, crtc_state)) { - ret = drm_atomic_add_affected_connectors(state, -
[PATCH 4/4] drm/i915: Use drm_modeset_lock_ctx_retry() & co.
From: Ville Syrjälä We have the modeset lock dance hand rolled in quite a few places. Use drm_modeset_lock_ctx_retry() and drm_modeset_lock_all_ctx_retry() to simplify our lives. Cc: Sean Paul Cc: Daniel Vetter Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/g4x_dp.c | 17 +-- drivers/gpu/drm/i915/display/intel_audio.c| 17 +-- drivers/gpu/drm/i915/display/intel_ddi.c | 16 +-- drivers/gpu/drm/i915/display/intel_display.c | 102 ++ .../drm/i915/display/intel_display_debugfs.c | 45 +++- drivers/gpu/drm/i915/display/intel_pipe_crc.c | 38 +++ 6 files changed, 69 insertions(+), 166 deletions(-) diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c index de0f358184aa..68e7f4233fa9 100644 --- a/drivers/gpu/drm/i915/display/g4x_dp.c +++ b/drivers/gpu/drm/i915/display/g4x_dp.c @@ -1167,23 +1167,10 @@ intel_dp_hotplug(struct intel_encoder *encoder, state = intel_encoder_hotplug(encoder, connector); - drm_modeset_acquire_init(&ctx, 0); - - for (;;) { + drm_modeset_lock_ctx_retry(&ctx, NULL, 0, ret) ret = intel_dp_retrain_link(encoder, &ctx); - if (ret == -EDEADLK) { - drm_modeset_backoff(&ctx); - continue; - } - - break; - } - - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - drm_WARN(encoder->base.dev, ret, -"Acquiring modeset locks failed with %i\n", ret); + drm_WARN_ON(encoder->base.dev, ret); /* * Keeping it consistent with intel_ddi_hotplug() and diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index 5f4f316b3ab5..a3b6b00e6633 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -969,28 +969,17 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv, if (!crtc) return; - drm_modeset_acquire_init(&ctx, 0); state = drm_atomic_state_alloc(&dev_priv->drm); if (drm_WARN_ON(&dev_priv->drm, !state)) return; - state->acquire_ctx = &ctx; - -retry: - ret = glk_force_audio_cdclk_commit(to_intel_atomic_state(state), crtc, - enable); - if (ret == -EDEADLK) { - drm_atomic_state_clear(state); - drm_modeset_backoff(&ctx); - goto retry; - } + drm_modeset_lock_ctx_retry(&ctx, state, 0, ret) + ret = glk_force_audio_cdclk_commit(to_intel_atomic_state(state), + crtc, enable); drm_WARN_ON(&dev_priv->drm, ret); drm_atomic_state_put(state); - - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); } static unsigned long i915_audio_component_get_power(struct device *kdev) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 26a3aa73fcc4..9aa7369d8dbe 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -4210,26 +4210,14 @@ intel_ddi_hotplug(struct intel_encoder *encoder, state = intel_encoder_hotplug(encoder, connector); - drm_modeset_acquire_init(&ctx, 0); - - for (;;) { + drm_modeset_lock_ctx_retry(&ctx, NULL, 0, ret) { if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA) ret = intel_hdmi_reset_link(encoder, &ctx); else ret = intel_dp_retrain_link(encoder, &ctx); - - if (ret == -EDEADLK) { - drm_modeset_backoff(&ctx); - continue; - } - - break; } - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - drm_WARN(encoder->base.dev, ret, -"Acquiring modeset locks failed with %i\n", ret); + drm_WARN_ON(encoder->base.dev, ret); /* * Unpowered type-c dongles can take some time to boot and be diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 3718399c4c2f..6f5bc56d68e0 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -12057,40 +12057,30 @@ static void sanitize_watermarks(struct drm_i915_private *dev_priv) intel_state = to_intel_atomic_state(state); - drm_modeset_acquire_init(&ctx, 0); + drm_modeset_lock_ctx_retry(&ctx, state, 0, ret) { + /* +* Hardware readout is the only time we don't want to calculate +* intermediate watermarks (since we don't trust the current +* watermarks). +*/ + if (!HAS_GMCH
[PATCH 00/22] drm: Review of mode copies
From: Ville Syrjälä I might be taking this a bit too far, but the lack of consistency in our methods to copy drm_display_mode structs around is bugging me. The main worry is the embedded list head, which if clobbered could lead to list corruption. I'd also prefer to make sure even the valid list heads don't propagate between copies since that makes no sense. While going through some of the code I also spotted some very weird on stack copies being made for no reason at all. I elimininated a few of them here, but there could certainly be more lurking in the shadows. Cc: Abhinav Kumar Cc: Alain Volmat Cc: Alex Deucher Cc: amd-...@lists.freedesktop.org Cc: Andrzej Hajda Cc: Aurabindo Pillai Cc: Chen Feng Cc: Chun-Kuang Hu Cc: Emma Anholt Cc: freedr...@lists.freedesktop.org Cc: Harry Wentland Cc: "Heiko Stübner" Cc: Jernej Skrabec Cc: John Stultz Cc: Jonas Karlman Cc: Jyri Sarha Cc: Laurent Pinchart Cc: Leo Li Cc: linux-arm-ker...@lists.infradead.org Cc: linux-arm-...@vger.kernel.org Cc: linux-rockc...@lists.infradead.org Cc: Maxime Ripard Cc: Neil Armstrong Cc: Nikola Cornij Cc: Patrik Jakobsson Cc: Philipp Zabel Cc: Rob Clark Cc: Robert Foss Cc: Rodrigo Siqueira Cc: Sam Ravnborg Cc: Sandy Huang Cc: Sean Paul Cc: Thierry Reding Cc: Tian Tao Cc: Tomi Valkeinen Cc: Xinliang Liu Cc: Xinwei Kong Ville Syrjälä (22): drm: Add drm_mode_init() drm/amdgpu: Remove pointless on stack mode copies drm/amdgpu: Use drm_mode_init() for on-stack modes drm/amdgpu: Use drm_mode_copy() drm/radeon: Use drm_mode_copy() drm/bridge: Use drm_mode_copy() drm/gma500: Use drm_mode_copy() drm/hisilicon: Use drm_mode_init() for on-stack modes drm/imx: Use drm_mode_duplicate() drm/msm: Nuke weird on stack mode copy drm/msm: Use drm_mode_init() for on-stack modes drm/msm: Use drm_mode_copy() drm/mtk: Use drm_mode_init() for on-stack modes drm/rockchip: Use drm_mode_copy() drm/sti: Use drm_mode_copy() drm/tilcdc: Use drm_mode_copy() drm/vc4: Use drm_mode_copy() drm/i915: Use drm_mode_init() for on-stack modes drm/i915: Use drm_mode_copy() drm/panel: Use drm_mode_duplicate() drm: Use drm_mode_init() for on-stack modes drm: Use drm_mode_copy() .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 4 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 ++- drivers/gpu/drm/bridge/nwl-dsi.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/tc358767.c | 2 +- drivers/gpu/drm/drm_crtc_helper.c | 12 +++--- drivers/gpu/drm/drm_edid.c| 8 +++- drivers/gpu/drm/drm_modes.c | 21 +- drivers/gpu/drm/drm_vblank.c | 2 +- drivers/gpu/drm/gma500/oaktrail_crtc.c| 8 +--- drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 2 +- drivers/gpu/drm/i915/display/intel_display.c | 20 + drivers/gpu/drm/imx/imx-ldb.c | 3 +- drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++-- drivers/gpu/drm/msm/dp/dp_display.c | 2 +- drivers/gpu/drm/msm/dp/dp_drm.c | 10 ++--- drivers/gpu/drm/panel/panel-truly-nt35597.c | 3 +- .../gpu/drm/panel/panel-visionox-rm69299.c| 4 +- drivers/gpu/drm/radeon/radeon_connectors.c| 4 +- drivers/gpu/drm/rockchip/cdn-dp-core.c| 2 +- drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +- drivers/gpu/drm/rockchip/rk3066_hdmi.c| 2 +- drivers/gpu/drm/sti/sti_dvo.c | 2 +- drivers/gpu/drm/sti/sti_hda.c | 2 +- drivers/gpu/drm/sti/sti_hdmi.c| 2 +- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 5 +-- include/drm/drm_modes.h | 2 + 30 files changed, 105 insertions(+), 79 deletions(-) -- 2.34.1
[PATCH 01/22] drm: Add drm_mode_init()
From: Ville Syrjälä Add a variant of drm_mode_copy() that explicitly clears out the list head of the destination mode. Helpful to guarantee we don't have stack garbage left in there for on-stack modes. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modes.c | 17 + include/drm/drm_modes.h | 2 ++ 2 files changed, 19 insertions(+) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 96b13e36293c..40d4ce4a1da4 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -892,6 +892,23 @@ void drm_mode_copy(struct drm_display_mode *dst, const struct drm_display_mode * } EXPORT_SYMBOL(drm_mode_copy); +/** + * drm_mode_init - initialize the mode from another mode + * @dst: mode to overwrite + * @src: mode to copy + * + * Copy an existing mode into another mode, zeroing the + * list head of the destination mode. Typically used + * to guarantee the list head is not left with stack + * garbage in on-stack modes. + */ +void drm_mode_init(struct drm_display_mode *dst, const struct drm_display_mode *src) +{ + memset(dst, 0, sizeof(*dst)); + drm_mode_copy(dst, src); +} +EXPORT_SYMBOL(drm_mode_init); + /** * drm_mode_duplicate - allocate and duplicate an existing mode * @dev: drm_device to allocate the duplicated mode for diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 29ba4adf0c53..e6e5a588fab1 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -484,6 +484,8 @@ void drm_mode_set_crtcinfo(struct drm_display_mode *p, int adjust_flags); void drm_mode_copy(struct drm_display_mode *dst, const struct drm_display_mode *src); +void drm_mode_init(struct drm_display_mode *dst, + const struct drm_display_mode *src); struct drm_display_mode *drm_mode_duplicate(struct drm_device *dev, const struct drm_display_mode *mode); bool drm_mode_match(const struct drm_display_mode *mode1, -- 2.34.1
[PATCH 02/22] drm/amdgpu: Remove pointless on stack mode copies
From: Ville Syrjälä These on stack copies of the modes appear to be pointless. Just look at the originals directly. Cc: Harry Wentland Cc: Leo Li Cc: Rodrigo Siqueira Cc: Alex Deucher Cc: amd-...@lists.freedesktop.org Cc: Nikola Cornij Cc: Aurabindo Pillai Signed-off-by: Ville Syrjälä --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 +-- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 21dba337dab0..65aab0d086b6 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -10139,27 +10139,27 @@ static bool is_timing_unchanged_for_freesync(struct drm_crtc_state *old_crtc_state, struct drm_crtc_state *new_crtc_state) { - struct drm_display_mode old_mode, new_mode; + const struct drm_display_mode *old_mode, *new_mode; if (!old_crtc_state || !new_crtc_state) return false; - old_mode = old_crtc_state->mode; - new_mode = new_crtc_state->mode; + old_mode = &old_crtc_state->mode; + new_mode = &new_crtc_state->mode; - if (old_mode.clock == new_mode.clock && - old_mode.hdisplay== new_mode.hdisplay && - old_mode.vdisplay== new_mode.vdisplay && - old_mode.htotal == new_mode.htotal && - old_mode.vtotal != new_mode.vtotal && - old_mode.hsync_start == new_mode.hsync_start && - old_mode.vsync_start != new_mode.vsync_start && - old_mode.hsync_end == new_mode.hsync_end && - old_mode.vsync_end != new_mode.vsync_end && - old_mode.hskew == new_mode.hskew && - old_mode.vscan == new_mode.vscan && - (old_mode.vsync_end - old_mode.vsync_start) == - (new_mode.vsync_end - new_mode.vsync_start)) + if (old_mode->clock == new_mode->clock && + old_mode->hdisplay== new_mode->hdisplay && + old_mode->vdisplay== new_mode->vdisplay && + old_mode->htotal == new_mode->htotal && + old_mode->vtotal != new_mode->vtotal && + old_mode->hsync_start == new_mode->hsync_start && + old_mode->vsync_start != new_mode->vsync_start && + old_mode->hsync_end == new_mode->hsync_end && + old_mode->vsync_end != new_mode->vsync_end && + old_mode->hskew == new_mode->hskew && + old_mode->vscan == new_mode->vscan && + (old_mode->vsync_end - old_mode->vsync_start) == + (new_mode->vsync_end - new_mode->vsync_start)) return true; return false; -- 2.34.1
[PATCH 03/22] drm/amdgpu: Use drm_mode_init() for on-stack modes
From: Ville Syrjälä Initialize on-stack modes with drm_mode_init() to guarantee no stack garbage in the list head, or that we aren't copying over another mode's list head. Based on the following cocci script, with manual fixups: @decl@ identifier M; expression E; @@ - struct drm_display_mode M = E; + struct drm_display_mode M; @@ identifier decl.M; expression decl.E; statement S, S1; @@ struct drm_display_mode M; ... when != S + drm_mode_init(&M, &E); + S1 @@ expression decl.E; @@ - &*E + E Cc: Harry Wentland Cc: Leo Li Cc: Rodrigo Siqueira Cc: Alex Deucher Cc: amd-...@lists.freedesktop.org Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 65aab0d086b6..bd23c9e481eb 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6361,7 +6361,7 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector, const struct drm_connector_state *con_state = dm_state ? &dm_state->base : NULL; struct dc_stream_state *stream = NULL; - struct drm_display_mode mode = *drm_mode; + struct drm_display_mode mode; struct drm_display_mode saved_mode; struct drm_display_mode *freesync_mode = NULL; bool native_mode_found = false; @@ -6374,6 +6374,7 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector, #endif struct dc_sink *sink = NULL; + drm_mode_init(&mode, drm_mode); memset(&saved_mode, 0, sizeof(saved_mode)); if (aconnector == NULL) { -- 2.34.1
[PATCH 05/22] drm/radeon: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Cc: Alex Deucher Cc: amd-...@lists.freedesktop.org Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/radeon/radeon_connectors.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index a7925a8290b2..0cb1345c6ba4 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -777,7 +777,7 @@ static void radeon_fixup_lvds_native_mode(struct drm_encoder *encoder, if (mode->type & DRM_MODE_TYPE_PREFERRED) { if (mode->hdisplay != native_mode->hdisplay || mode->vdisplay != native_mode->vdisplay) - memcpy(native_mode, mode, sizeof(*mode)); + drm_mode_copy(native_mode, mode); } } @@ -786,7 +786,7 @@ static void radeon_fixup_lvds_native_mode(struct drm_encoder *encoder, list_for_each_entry_safe(mode, t, &connector->probed_modes, head) { if (mode->hdisplay == native_mode->hdisplay && mode->vdisplay == native_mode->vdisplay) { - *native_mode = *mode; + drm_mode_copy(native_mode, mode); drm_mode_set_crtcinfo(native_mode, CRTC_INTERLACE_HALVE_V); DRM_DEBUG_KMS("Determined LVDS native mode details from EDID\n"); break; -- 2.34.1
[PATCH 07/22] drm/gma500: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Cc: Patrik Jakobsson Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/gma500/oaktrail_crtc.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c b/drivers/gpu/drm/gma500/oaktrail_crtc.c index 36c7c2686c90..79fc602b35bc 100644 --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c @@ -385,12 +385,8 @@ static int oaktrail_crtc_mode_set(struct drm_crtc *crtc, if (!gma_power_begin(dev, true)) return 0; - memcpy(&gma_crtc->saved_mode, - mode, - sizeof(struct drm_display_mode)); - memcpy(&gma_crtc->saved_adjusted_mode, - adjusted_mode, - sizeof(struct drm_display_mode)); + drm_mode_copy(&gma_crtc->saved_mode, mode); + drm_mode_copy(&gma_crtc->saved_adjusted_mode, adjusted_mode); list_for_each_entry(connector, &mode_config->connector_list, head) { if (!connector->encoder || connector->encoder->crtc != crtc) -- 2.34.1
[PATCH 06/22] drm/bridge: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Cc: Andrzej Hajda Cc: Neil Armstrong Cc: Robert Foss Cc: Laurent Pinchart Cc: Jonas Karlman Cc: Jernej Skrabec Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/bridge/nwl-dsi.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/tc358767.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c index 963a6794735f..881cf338d5cf 100644 --- a/drivers/gpu/drm/bridge/nwl-dsi.c +++ b/drivers/gpu/drm/bridge/nwl-dsi.c @@ -857,7 +857,7 @@ nwl_dsi_bridge_mode_set(struct drm_bridge *bridge, /* Save the new desired phy config */ memcpy(&dsi->phy_cfg, &new_cfg, sizeof(new_cfg)); - memcpy(&dsi->mode, adjusted_mode, sizeof(dsi->mode)); + drm_mode_copy(&dsi->mode, adjusted_mode); drm_mode_debug_printmodeline(adjusted_mode); if (pm_runtime_resume_and_get(dev) < 0) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 4befc104d220..a563460f8d20 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2830,7 +2830,7 @@ static void dw_hdmi_bridge_mode_set(struct drm_bridge *bridge, mutex_lock(&hdmi->mutex); /* Store the display mode for plugin/DKMS poweron events */ - memcpy(&hdmi->previous_mode, mode, sizeof(hdmi->previous_mode)); + drm_mode_copy(&hdmi->previous_mode, mode); mutex_unlock(&hdmi->mutex); } diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index c23e0abc65e8..7f9574b17caa 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -1312,7 +1312,7 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge, { struct tc_data *tc = bridge_to_tc(bridge); - tc->mode = *mode; + drm_mode_copy(&tc->mode, mode); } static struct edid *tc_get_edid(struct drm_bridge *bridge, -- 2.34.1
[PATCH 08/22] drm/hisilicon: Use drm_mode_init() for on-stack modes
From: Ville Syrjälä Initialize on-stack modes with drm_mode_init() to guarantee no stack garbage in the list head. Cc: Xinliang Liu Cc: Tian Tao Cc: John Stultz Cc: Xinwei Kong Cc: Chen Feng Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c index 1d556482bb46..53bd2dbf38cd 100644 --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c @@ -657,7 +657,7 @@ static enum drm_mode_status dsi_encoder_mode_valid(struct drm_encoder *encoder, * reset adj_mode to the mode value each time, * so we don't adjust the mode twice */ - drm_mode_copy(&adj_mode, mode); + drm_mode_init(&adj_mode, mode); crtc_funcs = crtc->helper_private; if (crtc_funcs && crtc_funcs->mode_fixup) -- 2.34.1
[PATCH 04/22] drm/amdgpu: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Cc: Alex Deucher Cc: Harry Wentland Cc: Leo Li Cc: Rodrigo Siqueira Cc: amd-...@lists.freedesktop.org Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c| 4 ++-- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c index fa20261aa928..673078faa27a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c @@ -626,7 +626,7 @@ amdgpu_connector_fixup_lcd_native_mode(struct drm_encoder *encoder, if (mode->type & DRM_MODE_TYPE_PREFERRED) { if (mode->hdisplay != native_mode->hdisplay || mode->vdisplay != native_mode->vdisplay) - memcpy(native_mode, mode, sizeof(*mode)); + drm_mode_copy(native_mode, mode); } } @@ -635,7 +635,7 @@ amdgpu_connector_fixup_lcd_native_mode(struct drm_encoder *encoder, list_for_each_entry_safe(mode, t, &connector->probed_modes, head) { if (mode->hdisplay == native_mode->hdisplay && mode->vdisplay == native_mode->vdisplay) { - *native_mode = *mode; + drm_mode_copy(native_mode, mode); drm_mode_set_crtcinfo(native_mode, CRTC_INTERLACE_HALVE_V); DRM_DEBUG_KMS("Determined LVDS native mode details from EDID\n"); break; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index bd23c9e481eb..514280699ad5 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6318,7 +6318,7 @@ get_highest_refresh_rate_mode(struct amdgpu_dm_connector *aconnector, } } - aconnector->freesync_vid_base = *m_pref; + drm_mode_copy(&aconnector->freesync_vid_base, m_pref); return m_pref; } @@ -6432,8 +6432,8 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector, recalculate_timing = is_freesync_video_mode(&mode, aconnector); if (recalculate_timing) { freesync_mode = get_highest_refresh_rate_mode(aconnector, false); - saved_mode = mode; - mode = *freesync_mode; + drm_mode_copy(&saved_mode, &mode); + drm_mode_copy(&mode, freesync_mode); } else { decide_crtc_timing_for_drm_display_mode( &mode, preferred_mode, scale); -- 2.34.1
[PATCH 10/22] drm/msm: Nuke weird on stack mode copy
From: Ville Syrjälä This on stack middle man mode looks entirely pointless. Just duplicate the original mode directly. Cc: Rob Clark Cc: Sean Paul Cc: Abhinav Kumar Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/msm/dp/dp_drm.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c index d4d360d19eba..09188d02aa1e 100644 --- a/drivers/gpu/drm/msm/dp/dp_drm.c +++ b/drivers/gpu/drm/msm/dp/dp_drm.c @@ -56,7 +56,7 @@ static int dp_connector_get_modes(struct drm_connector *connector) int rc = 0; struct msm_dp *dp; struct dp_display_mode *dp_mode = NULL; - struct drm_display_mode *m, drm_mode; + struct drm_display_mode *m; if (!connector) return 0; @@ -82,13 +82,11 @@ static int dp_connector_get_modes(struct drm_connector *connector) return rc; } if (dp_mode->drm_mode.clock) { /* valid DP mode */ - memset(&drm_mode, 0x0, sizeof(drm_mode)); - drm_mode_copy(&drm_mode, &dp_mode->drm_mode); - m = drm_mode_duplicate(connector->dev, &drm_mode); + m = drm_mode_duplicate(connector->dev, &dp_mode->drm_mode); if (!m) { DRM_ERROR("failed to add mode %ux%u\n", - drm_mode.hdisplay, - drm_mode.vdisplay); + dp_mode->drm_mode.hdisplay, + dp_mode->drm_mode.vdisplay); kfree(dp_mode); return 0; } -- 2.34.1
[PATCH 09/22] drm/imx: Use drm_mode_duplicate()
From: Ville Syrjälä Replace the hand rolled drm_mode_duplicate() with the real thing. @is_dup@ @@ drm_mode_duplicate(...) { ... } @depends on !is_dup@ expression dev, oldmode; identifier newmode; @@ - newmode = drm_mode_create(dev); + newmode = drm_mode_duplicate(dev, oldmode); ... - drm_mode_copy(newmode, oldmode); Cc: Philipp Zabel Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/imx/imx-ldb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index e5078d03020d..c8cf819f39ce 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -150,10 +150,9 @@ static int imx_ldb_connector_get_modes(struct drm_connector *connector) if (imx_ldb_ch->mode_valid) { struct drm_display_mode *mode; - mode = drm_mode_create(connector->dev); + mode = drm_mode_duplicate(connector->dev, &imx_ldb_ch->mode); if (!mode) return -EINVAL; - drm_mode_copy(mode, &imx_ldb_ch->mode); mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; drm_mode_probed_add(connector, mode); num_modes++; -- 2.34.1
[PATCH 11/22] drm/msm: Use drm_mode_init() for on-stack modes
From: Ville Syrjälä Initialize on-stack modes with drm_mode_init() to guarantee no stack garbage in the list head, or that we aren't copying over another mode's list head. Based on the following cocci script, with manual fixups: @decl@ identifier M; expression E; @@ - struct drm_display_mode M = E; + struct drm_display_mode M; @@ identifier decl.M; expression decl.E; statement S, S1; @@ struct drm_display_mode M; ... when != S + drm_mode_init(&M, &E); + S1 @@ expression decl.E; @@ - &*E + E Cc: Rob Clark Cc: Sean Paul Cc: Abhinav Kumar Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index ddd9d89cd456..e7813c6f7bd9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -248,12 +248,13 @@ static void dpu_encoder_phys_vid_setup_timing_engine( unsigned long lock_flags; struct dpu_hw_intf_cfg intf_cfg = { 0 }; + drm_mode_init(&mode, &phys_enc->cached_mode); + if (!phys_enc->hw_ctl->ops.setup_intf_cfg) { DPU_ERROR("invalid encoder %d\n", phys_enc != NULL); return; } - mode = phys_enc->cached_mode; if (!phys_enc->hw_intf->ops.setup_timing_gen) { DPU_ERROR("timing engine setup is not supported\n"); return; @@ -652,7 +653,9 @@ static int dpu_encoder_phys_vid_get_frame_count( { struct intf_status s = {0}; u32 fetch_start = 0; - struct drm_display_mode mode = phys_enc->cached_mode; + struct drm_display_mode mode; + + drm_mode_init(&mode, &phys_enc->cached_mode); if (!dpu_encoder_phys_vid_is_master(phys_enc)) return -EINVAL; -- 2.34.1
[PATCH 12/22] drm/msm: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Cc: Rob Clark Cc: Sean Paul Cc: Abhinav Kumar Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +- drivers/gpu/drm/msm/dp/dp_display.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 34a6940d12c5..57592052af23 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -157,7 +157,7 @@ static void dpu_encoder_phys_cmd_mode_set( DPU_ERROR("invalid args\n"); return; } - phys_enc->cached_mode = *adj_mode; + drm_mode_copy(&phys_enc->cached_mode, adj_mode); DPU_DEBUG_CMDENC(cmd_enc, "caching mode:\n"); drm_mode_debug_printmodeline(adj_mode); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index e7813c6f7bd9..d5deca07b65a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -370,7 +370,7 @@ static void dpu_encoder_phys_vid_mode_set( struct dpu_encoder_irq *irq; if (adj_mode) { - phys_enc->cached_mode = *adj_mode; + drm_mode_copy(&phys_enc->cached_mode, adj_mode); drm_mode_debug_printmodeline(adj_mode); DPU_DEBUG_VIDENC(phys_enc, "caching mode:\n"); } diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 7cc4d21f2091..2ed6028ca8d6 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -825,7 +825,7 @@ static int dp_display_set_mode(struct msm_dp *dp_display, dp = container_of(dp_display, struct dp_display_private, dp_display); - dp->panel->dp_mode.drm_mode = mode->drm_mode; + drm_mode_copy(&dp->panel->dp_mode.drm_mode, &mode->drm_mode); dp->panel->dp_mode.bpp = mode->bpp; dp->panel->dp_mode.capabilities = mode->capabilities; dp_panel_init_panel_info(dp->panel); -- 2.34.1
[PATCH 13/22] drm/mtk: Use drm_mode_init() for on-stack modes
From: Ville Syrjälä Initialize on-stack modes with drm_mode_init() to guarantee no stack garbage in the list head. Based on the following cocci script, with manual fixups: @decl@ identifier M; expression E; @@ - struct drm_display_mode M = E; + struct drm_display_mode M; @@ identifier decl.M; expression decl.E; statement S, S1; @@ struct drm_display_mode M; ... when != S + drm_mode_init(&M, &E); + S1 @@ expression decl.E; @@ - &*E + E Cc: Chun-Kuang Hu Cc: Philipp Zabel Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 3196189429bc..30f879e68a2b 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c @@ -1218,7 +1218,7 @@ static int mtk_hdmi_bridge_mode_valid(struct drm_bridge *bridge, if (next_bridge) { struct drm_display_mode adjusted_mode; - drm_mode_copy(&adjusted_mode, mode); + drm_mode_init(&adjusted_mode, mode); if (!drm_bridge_chain_mode_fixup(next_bridge, mode, &adjusted_mode)) return MODE_BAD; -- 2.34.1
[PATCH 14/22] drm/rockchip: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Signed-off-by: Ville Syrjälä Cc: Sandy Huang Cc: "Heiko Stübner" Cc: linux-arm-ker...@lists.infradead.org Cc: linux-rockc...@lists.infradead.org --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +- drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +- drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 4740cc14beb8..adf1027a3f20 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -564,7 +564,7 @@ static void cdn_dp_encoder_mode_set(struct drm_encoder *encoder, video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC); video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC); - memcpy(&dp->mode, adjusted, sizeof(*mode)); + drm_mode_copy(&dp->mode, adjusted); } static bool cdn_dp_check_link_status(struct cdn_dp_device *dp) diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c index 046e8ec2a71c..740196d30fba 100644 --- a/drivers/gpu/drm/rockchip/inno_hdmi.c +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c @@ -488,7 +488,7 @@ static void inno_hdmi_encoder_mode_set(struct drm_encoder *encoder, inno_hdmi_setup(hdmi, adj_mode); /* Store the display mode for plugin/DPMS poweron events */ - memcpy(&hdmi->previous_mode, adj_mode, sizeof(hdmi->previous_mode)); + drm_mode_copy(&hdmi->previous_mode, adj_mode); } static void inno_hdmi_encoder_enable(struct drm_encoder *encoder) diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c index 1c546c3a8998..17e7c40a9e7b 100644 --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c @@ -383,7 +383,7 @@ rk3066_hdmi_encoder_mode_set(struct drm_encoder *encoder, struct rk3066_hdmi *hdmi = to_rk3066_hdmi(encoder); /* Store the display mode for plugin/DPMS poweron events. */ - memcpy(&hdmi->previous_mode, adj_mode, sizeof(hdmi->previous_mode)); + drm_mode_copy(&hdmi->previous_mode, adj_mode); } static void rk3066_hdmi_encoder_enable(struct drm_encoder *encoder) -- 2.34.1
[PATCH 17/22] drm/vc4: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Cc: Emma Anholt Cc: Maxime Ripard Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/vc4/vc4_hdmi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 5ac3216f2d4a..6c58b0fd13fb 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1234,9 +1234,8 @@ static void vc4_hdmi_encoder_atomic_mode_set(struct drm_encoder *encoder, struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); mutex_lock(&vc4_hdmi->mutex); - memcpy(&vc4_hdmi->saved_adjusted_mode, - &crtc_state->adjusted_mode, - sizeof(vc4_hdmi->saved_adjusted_mode)); + drm_mode_copy(&vc4_hdmi->saved_adjusted_mode, + &crtc_state->adjusted_mode); mutex_unlock(&vc4_hdmi->mutex); } -- 2.34.1
[PATCH 19/22] drm/i915: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 74c5a99ab276..661e36435793 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5506,8 +5506,10 @@ intel_crtc_copy_uapi_to_hw_state_modeset(struct intel_atomic_state *state, crtc_state->hw.enable = crtc_state->uapi.enable; crtc_state->hw.active = crtc_state->uapi.active; - crtc_state->hw.mode = crtc_state->uapi.mode; - crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode; + drm_mode_copy(&crtc_state->hw.mode, + &crtc_state->uapi.mode); + drm_mode_copy(&crtc_state->hw.adjusted_mode, + &crtc_state->uapi.adjusted_mode); crtc_state->hw.scaling_filter = crtc_state->uapi.scaling_filter; intel_crtc_copy_uapi_to_hw_state_nomodeset(state, crtc); @@ -5584,9 +5586,12 @@ copy_bigjoiner_crtc_state_modeset(struct intel_atomic_state *state, memset(&slave_crtc_state->hw, 0, sizeof(slave_crtc_state->hw)); slave_crtc_state->hw.enable = master_crtc_state->hw.enable; slave_crtc_state->hw.active = master_crtc_state->hw.active; - slave_crtc_state->hw.mode = master_crtc_state->hw.mode; - slave_crtc_state->hw.pipe_mode = master_crtc_state->hw.pipe_mode; - slave_crtc_state->hw.adjusted_mode = master_crtc_state->hw.adjusted_mode; + drm_mode_copy(&slave_crtc_state->hw.mode, + &master_crtc_state->hw.mode); + drm_mode_copy(&slave_crtc_state->hw.pipe_mode, + &master_crtc_state->hw.pipe_mode); + drm_mode_copy(&slave_crtc_state->hw.adjusted_mode, + &master_crtc_state->hw.adjusted_mode); slave_crtc_state->hw.scaling_filter = master_crtc_state->hw.scaling_filter; copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc); -- 2.34.1
[PATCH 20/22] drm/panel: Use drm_mode_duplicate()
From: Ville Syrjälä Replace the hand rolled drm_mode_duplicate() with the real thing. @is_dup@ @@ drm_mode_duplicate(...) { ... } @depends on !is_dup@ expression dev, oldmode; identifier newmode; @@ - newmode = drm_mode_create(dev); + newmode = drm_mode_duplicate(dev, oldmode); ... - drm_mode_copy(newmode, oldmode); Signed-off-by: Ville Syrjälä Cc: Thierry Reding Cc: Sam Ravnborg --- drivers/gpu/drm/panel/panel-truly-nt35597.c| 3 +-- drivers/gpu/drm/panel/panel-visionox-rm69299.c | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-truly-nt35597.c b/drivers/gpu/drm/panel/panel-truly-nt35597.c index b24b92d93ea5..9ca5c7ff41d6 100644 --- a/drivers/gpu/drm/panel/panel-truly-nt35597.c +++ b/drivers/gpu/drm/panel/panel-truly-nt35597.c @@ -446,7 +446,7 @@ static int truly_nt35597_get_modes(struct drm_panel *panel, const struct nt35597_config *config; config = ctx->config; - mode = drm_mode_create(connector->dev); + mode = drm_mode_duplicate(connector->dev, config->dm); if (!mode) { dev_err(ctx->dev, "failed to create a new display mode\n"); return 0; @@ -454,7 +454,6 @@ static int truly_nt35597_get_modes(struct drm_panel *panel, connector->display_info.width_mm = config->width_mm; connector->display_info.height_mm = config->height_mm; - drm_mode_copy(mode, config->dm); mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; drm_mode_probed_add(connector, mode); diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c b/drivers/gpu/drm/panel/panel-visionox-rm69299.c index eb43503ec97b..db2443ac81d3 100644 --- a/drivers/gpu/drm/panel/panel-visionox-rm69299.c +++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c @@ -168,7 +168,8 @@ static int visionox_rm69299_get_modes(struct drm_panel *panel, struct visionox_rm69299 *ctx = panel_to_ctx(panel); struct drm_display_mode *mode; - mode = drm_mode_create(connector->dev); + mode = drm_mode_duplicate(connector->dev, + &visionox_rm69299_1080x2248_60hz); if (!mode) { dev_err(ctx->panel.dev, "failed to create a new display mode\n"); return 0; @@ -176,7 +177,6 @@ static int visionox_rm69299_get_modes(struct drm_panel *panel, connector->display_info.width_mm = 74; connector->display_info.height_mm = 131; - drm_mode_copy(mode, &visionox_rm69299_1080x2248_60hz); mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; drm_mode_probed_add(connector, mode); -- 2.34.1
[PATCH 22/22] drm: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_crtc_helper.c | 4 ++-- drivers/gpu/drm/drm_vblank.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index a34aa009725f..b632825654a9 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -305,7 +305,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, /* Update crtc values up front so the driver can rely on them for mode * setting. */ - crtc->mode = *mode; + drm_mode_copy(&crtc->mode, mode); crtc->x = x; crtc->y = y; @@ -341,7 +341,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, } DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); - crtc->hwmode = *adjusted_mode; + drm_mode_copy(&crtc->hwmode, adjusted_mode); /* Prepare the encoders and CRTCs before setting the mode. */ drm_for_each_encoder(encoder, dev) { diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index b701cda86d0c..2ff31717a3de 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -644,7 +644,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc, vblank->linedur_ns = linedur_ns; vblank->framedur_ns = framedur_ns; - vblank->hwmode = *mode; + drm_mode_copy(&vblank->hwmode, mode); drm_dbg_core(dev, "crtc %u: hwmode: htotal %d, vtotal %d, vdisplay %d\n", -- 2.34.1
[PATCH 15/22] drm/sti: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Cc: Alain Volmat Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/sti/sti_dvo.c | 2 +- drivers/gpu/drm/sti/sti_hda.c | 2 +- drivers/gpu/drm/sti/sti_hdmi.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c index b6ee8a82e656..f3a5616b7daf 100644 --- a/drivers/gpu/drm/sti/sti_dvo.c +++ b/drivers/gpu/drm/sti/sti_dvo.c @@ -288,7 +288,7 @@ static void sti_dvo_set_mode(struct drm_bridge *bridge, DRM_DEBUG_DRIVER("\n"); - memcpy(&dvo->mode, mode, sizeof(struct drm_display_mode)); + drm_mode_copy(&dvo->mode, mode); /* According to the path used (main or aux), the dvo clocks should * have a different parent clock. */ diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c index 03f3377f918c..c9e6db15ab66 100644 --- a/drivers/gpu/drm/sti/sti_hda.c +++ b/drivers/gpu/drm/sti/sti_hda.c @@ -523,7 +523,7 @@ static void sti_hda_set_mode(struct drm_bridge *bridge, DRM_DEBUG_DRIVER("\n"); - memcpy(&hda->mode, mode, sizeof(struct drm_display_mode)); + drm_mode_copy(&hda->mode, mode); if (!hda_get_mode_idx(hda->mode, &mode_idx)) { DRM_ERROR("Undefined mode\n"); diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index f3ace11209dd..bb2a2868de2d 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -940,7 +940,7 @@ static void sti_hdmi_set_mode(struct drm_bridge *bridge, DRM_DEBUG_DRIVER("\n"); /* Copy the drm display mode in the connector local structure */ - memcpy(&hdmi->mode, mode, sizeof(struct drm_display_mode)); + drm_mode_copy(&hdmi->mode, mode); /* Update clock framerate according to the selected mode */ ret = clk_set_rate(hdmi->clk_pix, mode->clock * 1000); -- 2.34.1
[PATCH 16/22] drm/tilcdc: Use drm_mode_copy()
From: Ville Syrjälä struct drm_display_mode embeds a list head, so overwriting the full struct with another one will corrupt the list (if the destination mode is on a list). Use drm_mode_copy() instead which explicitly preserves the list head of the destination mode. Even if we know the destination mode is not on any list using drm_mode_copy() seems decent as it sets a good example. Bad examples of not using it might eventually get copied into code where preserving the list head actually matters. Obviously one case not covered here is when the mode itself is embedded in a larger structure and the whole structure is copied. But if we are careful when copying into modes embedded in structures I think we can be a little more reassured that bogus list heads haven't been propagated in. @is_mode_copy@ @@ drm_mode_copy(...) { ... } @depends on !is_mode_copy@ struct drm_display_mode *mode; expression E, S; @@ ( - *mode = E + drm_mode_copy(mode, &E) | - memcpy(mode, E, S) + drm_mode_copy(mode, E) ) @depends on !is_mode_copy@ struct drm_display_mode mode; expression E; @@ ( - mode = E + drm_mode_copy(&mode, &E) | - memcpy(&mode, E, S) + drm_mode_copy(&mode, E) ) @@ struct drm_display_mode *mode; @@ - &*mode + mode Cc: Jyri Sarha Cc: Tomi Valkeinen Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c index 29890d704cb4..853c6b443fff 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c @@ -433,7 +433,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc) set_scanout(crtc, fb); - crtc->hwmode = crtc->state->adjusted_mode; + drm_mode_copy(&crtc->hwmode, &crtc->state->adjusted_mode); tilcdc_crtc->hvtotal_us = tilcdc_mode_hvtotal(&crtc->hwmode); -- 2.34.1
[PATCH 21/22] drm: Use drm_mode_init() for on-stack modes
From: Ville Syrjälä Initialize on-stack modes with drm_mode_init() to guarantee no stack garbage in the list head, or that we aren't copying over another mode's list head. Based on the following cocci script, with manual fixups: @decl@ identifier M; expression E; @@ - struct drm_display_mode M = E; + struct drm_display_mode M; @@ identifier decl.M; expression decl.E; statement S, S1; @@ struct drm_display_mode M; ... when != S + drm_mode_init(&M, &E); + S1 @@ expression decl.E; @@ - &*E + E Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_crtc_helper.c | 8 drivers/gpu/drm/drm_edid.c| 8 ++-- drivers/gpu/drm/drm_modes.c | 4 +++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index bff917531f33..a34aa009725f 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -297,8 +297,8 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, return false; } - saved_mode = crtc->mode; - saved_hwmode = crtc->hwmode; + drm_mode_init(&saved_mode, &crtc->mode); + drm_mode_init(&saved_hwmode, &crtc->hwmode); saved_x = crtc->x; saved_y = crtc->y; @@ -411,8 +411,8 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, drm_mode_destroy(dev, adjusted_mode); if (!ret) { crtc->enabled = saved_enabled; - crtc->mode = saved_mode; - crtc->hwmode = saved_hwmode; + drm_mode_copy(&crtc->mode, &saved_mode); + drm_mode_copy(&crtc->hwmode, &saved_hwmode); crtc->x = saved_x; crtc->y = saved_y; } diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index a7663f9a11d2..1156bfeabaf6 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3476,9 +3476,11 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; for (vic = 1; vic < cea_num_vics(); vic = cea_next_vic(vic)) { - struct drm_display_mode cea_mode = *cea_mode_for_vic(vic); + struct drm_display_mode cea_mode; unsigned int clock1, clock2; + drm_mode_init(&cea_mode, cea_mode_for_vic(vic)); + /* Check both 60Hz and 59.94Hz */ clock1 = cea_mode.clock; clock2 = cea_mode_alternate_clock(&cea_mode); @@ -3515,9 +3517,11 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; for (vic = 1; vic < cea_num_vics(); vic = cea_next_vic(vic)) { - struct drm_display_mode cea_mode = *cea_mode_for_vic(vic); + struct drm_display_mode cea_mode; unsigned int clock1, clock2; + drm_mode_init(&cea_mode, cea_mode_for_vic(vic)); + /* Check both 60Hz and 59.94Hz */ clock1 = cea_mode.clock; clock2 = cea_mode_alternate_clock(&cea_mode); diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 40d4ce4a1da4..86904d082ff2 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -788,7 +788,9 @@ EXPORT_SYMBOL(drm_mode_vrefresh); void drm_mode_get_hv_timing(const struct drm_display_mode *mode, int *hdisplay, int *vdisplay) { - struct drm_display_mode adjusted = *mode; + struct drm_display_mode adjusted; + + drm_mode_init(&adjusted, mode); drm_mode_set_crtcinfo(&adjusted, CRTC_STEREO_DOUBLE_ONLY); *hdisplay = adjusted.crtc_hdisplay; -- 2.34.1
[PATCH 18/22] drm/i915: Use drm_mode_init() for on-stack modes
From: Ville Syrjälä Initialize on-stack modes with drm_mode_init() to guarantee no stack garbage in the list head, or that we aren't copying over another mode's list head. Based on the following cocci script, with manual fixups: @decl@ identifier M; expression E; @@ - struct drm_display_mode M = E; + struct drm_display_mode M; @@ identifier decl.M; expression decl.E; statement S, S1; @@ struct drm_display_mode M; ... when != S + drm_mode_init(&M, &E); + S1 @@ expression decl.E; @@ - &*E + E Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 740620ef07ad..74c5a99ab276 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6898,8 +6898,9 @@ intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - struct drm_display_mode adjusted_mode = - crtc_state->hw.adjusted_mode; + struct drm_display_mode adjusted_mode; + + drm_mode_init(&adjusted_mode, &crtc_state->hw.adjusted_mode); if (crtc_state->vrr.enable) { adjusted_mode.crtc_vtotal = crtc_state->vrr.vmax; -- 2.34.1
[PATCH] drm: Fix scaling_mode docs
From: Ville Syrjälä Fix the bad copy-pasta in the scaling_mode docs. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_connector.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index e0a30e0ee86a..3bc782b630b9 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1620,7 +1620,7 @@ EXPORT_SYMBOL(drm_mode_create_tv_properties); * connectors. * * Atomic drivers should use drm_connector_attach_scaling_mode_property() - * instead to correctly assign &drm_connector_state.picture_aspect_ratio + * instead to correctly assign &drm_connector_state.scaling_mode * in the atomic state. */ int drm_mode_create_scaling_mode_property(struct drm_device *dev) @@ -1740,7 +1740,7 @@ EXPORT_SYMBOL(drm_connector_attach_vrr_capable_property); * @scaling_mode_mask: or'ed mask of BIT(%DRM_MODE_SCALE_\*). * * This is used to add support for scaling mode to atomic drivers. - * The scaling mode will be set to &drm_connector_state.picture_aspect_ratio + * The scaling mode will be set to &drm_connector_state.scaling_mode * and can be used from &drm_connector_helper_funcs->atomic_check for validation. * * This is the atomic version of drm_mode_create_scaling_mode_property(). -- 2.32.0
[PATCH 0/4] drm/i915: Make the driver not oops on load on old machines
From: Ville Syrjälä Fix a pile of regression on older machines which just oops the driver on load. Cc: Dave Airlie Cc: Jani Nikula Cc: Maarten Lankhorst Cc: Thomas Hellström Cc: Chris Wilson Cc: Mika Kuoppala Ville Syrjälä (4): drm/i915: Replace the unconditional clflush with drm_clflush_virt_range() drm/i915: Convert unconditional clflush to drm_clflush_virt_range() drm/i915: Catch yet another unconditioal clflush drm/i915: Fix oops on platforms w/o hpd support drivers/gpu/drm/i915/display/intel_hotplug.c| 3 ++- drivers/gpu/drm/i915/gt/intel_ring_submission.c | 2 +- drivers/gpu/drm/i915/gt/intel_timeline.c| 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) -- 2.32.0
[PATCH 1/4] drm/i915: Replace the unconditional clflush with drm_clflush_virt_range()
From: Ville Syrjälä Not all machines have clflush, so don't go assuming they do. Not really sure why the clflush is even here since hwsp is supposed to get snooped I thought. Although in my case we're talking about a i830 machine where render/blitter snooping is definitely busted. But it might work for the hswp perhaps. Haven't really reverse engineered that one fully. Cc: sta...@vger.kernel.org Cc: Chris Wilson Cc: Mika Kuoppala Fixes: b436a5f8b6c8 ("drm/i915/gt: Track all timelines created using the HWSP") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/gt/intel_ring_submission.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c index 593524195707..586dca1731ce 100644 --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c @@ -292,7 +292,7 @@ static void xcs_sanitize(struct intel_engine_cs *engine) sanitize_hwsp(engine); /* And scrub the dirty cachelines for the HWSP */ - clflush_cache_range(engine->status_page.addr, PAGE_SIZE); + drm_clflush_virt_range(engine->status_page.addr, PAGE_SIZE); intel_engine_reset_pinned_contexts(engine); } -- 2.32.0
[PATCH 2/4] drm/i915: Convert unconditional clflush to drm_clflush_virt_range()
From: Ville Syrjälä This one is apparently a "clflush for good measure", so bit more justification (if you can call it that) than some of the others. Convert to drm_clflush_virt_range() again so that machines without clflush will survive the ordeal. Cc: sta...@vger.kernel.org Cc: Maarten Lankhorst Cc: Thomas Hellström #v1 Fixes: 12ca695d2c1e ("drm/i915: Do not share hwsp across contexts any more, v8.") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/gt/intel_timeline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c index 1257f4f11e66..23d7328892ed 100644 --- a/drivers/gpu/drm/i915/gt/intel_timeline.c +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c @@ -225,7 +225,7 @@ void intel_timeline_reset_seqno(const struct intel_timeline *tl) memset(hwsp_seqno + 1, 0, TIMELINE_SEQNO_BYTES - sizeof(*hwsp_seqno)); WRITE_ONCE(*hwsp_seqno, tl->seqno); - clflush(hwsp_seqno); + drm_clflush_virt_range(hwsp_seqno, TIMELINE_SEQNO_BYTES); } void intel_timeline_enter(struct intel_timeline *tl) -- 2.32.0
[PATCH 4/4] drm/i915: Fix oops on platforms w/o hpd support
From: Ville Syrjälä We don't have hpd support on i8xx/i915 which means hotplug_funcs==NULL. Let's not oops when loading the driver on one those machines. Cc: Dave Airlie Cc: Jani Nikula Fixes: cd030c7c11a4 ("drm/i915: constify hotplug function vtable.") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c index 3c1cec953b42..0e949a258a22 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c @@ -215,7 +215,8 @@ intel_hpd_irq_storm_switch_to_polling(struct drm_i915_private *dev_priv) static void intel_hpd_irq_setup(struct drm_i915_private *i915) { - if (i915->display_irqs_enabled && i915->hotplug_funcs->hpd_irq_setup) + if (i915->display_irqs_enabled && + i915->hotplug_funcs && i915->hotplug_funcs->hpd_irq_setup) i915->hotplug_funcs->hpd_irq_setup(i915); } -- 2.32.0
[PATCH 3/4] drm/i915: Catch yet another unconditioal clflush
From: Ville Syrjälä Replace the unconditional clflush() with drm_clflush_virt_range() which does the wbinvd() fallback when clflush is not available. This time no justification is given for the clflush in the offending commit. Cc: sta...@vger.kernel.org Cc: Maarten Lankhorst Cc: Thomas Hellström Fixes: 2c8ab3339e39 ("drm/i915: Pin timeline map after first timeline pin, v4.") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/gt/intel_timeline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c index 23d7328892ed..438bbc7b8147 100644 --- a/drivers/gpu/drm/i915/gt/intel_timeline.c +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c @@ -64,7 +64,7 @@ intel_timeline_pin_map(struct intel_timeline *timeline) timeline->hwsp_map = vaddr; timeline->hwsp_seqno = memset(vaddr + ofs, 0, TIMELINE_SEQNO_BYTES); - clflush(vaddr + ofs); + drm_clflush_virt_range(vaddr + ofs, TIMELINE_SEQNO_BYTES); return 0; } -- 2.32.0
[PATCH 1/2] drm: Always include the debugfs dentry in drm_crtc
From: Ville Syrjälä Remove the counterproductive CONFIG_DEBUG_FS ifdef and just include the debugfs dentry in drm_crtc always. This way we don't need annoying ifdefs in the actual code with DEBUGFS=n. Also we don't have these ifdefs around any of the other debugfs dentries either so can't see why drm_crtc should be special. This fixes the i915 DEBUGFS=n build because I assumed the dentry would always be there. Cc: Jani Nikula Reported-by: Nathan Chancellor Tested-by: Nathan Chancellor Fixes: e74c6aa955ca ("drm/i915/fbc: Register per-crtc debugfs files") Signed-off-by: Ville Syrjälä --- include/drm/drm_crtc.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 13eeba2a750a..4d01b4d89775 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1135,14 +1135,12 @@ struct drm_crtc { */ spinlock_t commit_lock; -#ifdef CONFIG_DEBUG_FS /** * @debugfs_entry: * * Debugfs directory for this CRTC. */ struct dentry *debugfs_entry; -#endif /** * @crc: -- 2.32.0
[PATCH 2/2] drm/dbi: Use a static inline stub for mipi_dbi_debugfs_init()
From: Ville Syrjälä Replace the slightly odd "#define NULL" thing with a standard static inline stub. Cc: Noralf Trønnes Cc: Sam Ravnborg Signed-off-by: Ville Syrjälä --- include/drm/drm_mipi_dbi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h index 05e194958265..6fe13cce2670 100644 --- a/include/drm/drm_mipi_dbi.h +++ b/include/drm/drm_mipi_dbi.h @@ -194,7 +194,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb, #ifdef CONFIG_DEBUG_FS void mipi_dbi_debugfs_init(struct drm_minor *minor); #else -#define mipi_dbi_debugfs_init NULL +static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {} #endif #endif /* __LINUX_MIPI_DBI_H */ -- 2.32.0
[PATCH 2/2] drm/i915: Skip SINK_COUNT read on CH7511
From: Ville Syrjälä CH7511 doesn't update SINK_COUNT properly so in order to detect the device as connected we have to ignore SINK_COUNT. In order to have access to the quirk list early enough we must move the drm_dp_read_desc() call to happen earlier. We can also skip re-reading this on eDP since we know it won't change. Cc: David S. Cc: Peteris Rudzusiks Tested-by: Peteris Rudzusiks Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105406 Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_dp.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 24b56b2a76c8..8fe6571cda8f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4242,8 +4242,14 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) if (!intel_dp_read_dpcd(intel_dp)) return false; - /* Don't clobber cached eDP rates. */ + /* +* Don't clobber cached eDP rates. Also skip re-reading +* the OUI/ID since we know it won't change. +*/ if (!intel_dp_is_edp(intel_dp)) { + drm_dp_read_desc(&intel_dp->aux, &intel_dp->desc, +drm_dp_is_branch(intel_dp->dpcd)); + intel_dp_set_sink_rates(intel_dp); intel_dp_set_common_rates(intel_dp); } @@ -4252,7 +4258,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) * Some eDP panels do not set a valid value for sink count, that is why * it don't care about read it here and in intel_edp_init_dpcd(). */ - if (!intel_dp_is_edp(intel_dp)) { + if (!intel_dp_is_edp(intel_dp) && + !drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_NO_SINK_COUNT)) { u8 count; ssize_t r; @@ -5586,9 +5593,6 @@ intel_dp_detect(struct drm_connector *connector, if (INTEL_GEN(dev_priv) >= 11) intel_dp_get_dsc_sink_cap(intel_dp); - drm_dp_read_desc(&intel_dp->aux, &intel_dp->desc, -drm_dp_is_branch(intel_dp->dpcd)); - intel_dp_configure_mst(intel_dp); if (intel_dp->is_mst) { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/dp: Add DP_DPCD_QUIRK_NO_SINK_COUNT
From: Ville Syrjälä CH7511 eDP->LVDS bridge doesn't seem to set SINK_COUNT properly causing i915 to detect it as disconnected. Add a quirk to ignore SINK_COUNT on these devices. Cc: David S. Cc: Peteris Rudzusiks Tested-by: Peteris Rudzusiks Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105406 Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_dp_helper.c | 4 +++- include/drm/drm_dp_helper.h | 7 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index e6af758a7d22..0b994d083a89 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -1280,7 +1280,9 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { /* LG LP140WF6-SPM1 eDP panel */ { OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) }, /* Apple panels need some additional handling to support PSR */ - { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) } + { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) }, + /* CH7511 seems to leave SINK_COUNT zeroed */ + { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) }, }; #undef OUI diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 3fc534ee8174..7e52eb81284a 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1414,6 +1414,13 @@ enum drm_dp_quirk { * driver still need to implement proper handling for such device. */ DP_DPCD_QUIRK_NO_PSR, + /** +* @DP_DPCD_QUIRK_NO_SINK_COUNT: +* +* The device does not set SINK_COUNT to a non-zero value. +* The driver should ignore SINK_COUNT during detection. +*/ + DP_DPCD_QUIRK_NO_SINK_COUNT, }; /** -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/edid: Clean up DRM_EDID_DIGITAL_* flags
From: Ville Syrjälä Give the "DFP 1.x" bit a proper name, and clean up the rest of the bits defines as well. Cc: Mario Kleiner Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_edid.c | 2 +- include/drm/drm_edid.h | 32 +--- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d87f574feeca..dd601ed6a30e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4570,7 +4570,7 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi * extensions which tell otherwise. */ if ((info->bpc == 0) && (edid->revision < 4) && - (edid->input & DRM_EDID_DIGITAL_TYPE_DVI)) { + (edid->input & DRM_EDID_DIGITAL_DFP_1_X)) { info->bpc = 8; DRM_DEBUG("%s: Assigning DFP sink color depth as %d bpc.\n", connector->name, info->bpc); diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 0e21e91c4314..88b63801f9db 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -177,21 +177,23 @@ struct detailed_timing { #define DRM_EDID_INPUT_BLANK_TO_BLACK (1 << 4) #define DRM_EDID_INPUT_VIDEO_LEVEL (3 << 5) #define DRM_EDID_INPUT_DIGITAL (1 << 7) -#define DRM_EDID_DIGITAL_DEPTH_MASK(7 << 4) -#define DRM_EDID_DIGITAL_DEPTH_UNDEF (0 << 4) -#define DRM_EDID_DIGITAL_DEPTH_6 (1 << 4) -#define DRM_EDID_DIGITAL_DEPTH_8 (2 << 4) -#define DRM_EDID_DIGITAL_DEPTH_10 (3 << 4) -#define DRM_EDID_DIGITAL_DEPTH_12 (4 << 4) -#define DRM_EDID_DIGITAL_DEPTH_14 (5 << 4) -#define DRM_EDID_DIGITAL_DEPTH_16 (6 << 4) -#define DRM_EDID_DIGITAL_DEPTH_RSVD(7 << 4) -#define DRM_EDID_DIGITAL_TYPE_UNDEF(0) -#define DRM_EDID_DIGITAL_TYPE_DVI (1) -#define DRM_EDID_DIGITAL_TYPE_HDMI_A (2) -#define DRM_EDID_DIGITAL_TYPE_HDMI_B (3) -#define DRM_EDID_DIGITAL_TYPE_MDDI (4) -#define DRM_EDID_DIGITAL_TYPE_DP (5) +#define DRM_EDID_DIGITAL_DEPTH_MASK(7 << 4) /* 1.4 */ +#define DRM_EDID_DIGITAL_DEPTH_UNDEF (0 << 4) /* 1.4 */ +#define DRM_EDID_DIGITAL_DEPTH_6 (1 << 4) /* 1.4 */ +#define DRM_EDID_DIGITAL_DEPTH_8 (2 << 4) /* 1.4 */ +#define DRM_EDID_DIGITAL_DEPTH_10 (3 << 4) /* 1.4 */ +#define DRM_EDID_DIGITAL_DEPTH_12 (4 << 4) /* 1.4 */ +#define DRM_EDID_DIGITAL_DEPTH_14 (5 << 4) /* 1.4 */ +#define DRM_EDID_DIGITAL_DEPTH_16 (6 << 4) /* 1.4 */ +#define DRM_EDID_DIGITAL_DEPTH_RSVD(7 << 4) /* 1.4 */ +#define DRM_EDID_DIGITAL_TYPE_MASK (7 << 0) /* 1.4 */ +#define DRM_EDID_DIGITAL_TYPE_UNDEF(0 << 0) /* 1.4 */ +#define DRM_EDID_DIGITAL_TYPE_DVI (1 << 0) /* 1.4 */ +#define DRM_EDID_DIGITAL_TYPE_HDMI_A (2 << 0) /* 1.4 */ +#define DRM_EDID_DIGITAL_TYPE_HDMI_B (3 << 0) /* 1.4 */ +#define DRM_EDID_DIGITAL_TYPE_MDDI (4 << 0) /* 1.4 */ +#define DRM_EDID_DIGITAL_TYPE_DP (5 << 0) /* 1.4 */ +#define DRM_EDID_DIGITAL_DFP_1_X (1 << 0) /* 1.3 */ #define DRM_EDID_FEATURE_DEFAULT_GTF (1 << 0) #define DRM_EDID_FEATURE_PREFERRED_TIMING (1 << 1) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/edid: Ignore "DFP 1.x" bit for EDID 1.2 and earlier
From: Ville Syrjälä From VESA EDID implementation guide v1.0: "For EDID version 1 revision 2 or earlier data structures when offset 14h bit 7 is set to one, the value of bits 6-0 are undefined, and therefore cannot be interpreted to mean anything." And since EDID 1.4 redefines that bit let's consult it only for EDID 1.3. Cc: Mario Kleiner Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_edid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index dd601ed6a30e..c3296a72fff9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4569,8 +4569,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi * tells us to assume 8 bpc color depth if the EDID doesn't have * extensions which tell otherwise. */ - if ((info->bpc == 0) && (edid->revision < 4) && - (edid->input & DRM_EDID_DIGITAL_DFP_1_X)) { + if (info->bpc == 0 && edid->revision == 3 && + edid->input & DRM_EDID_DIGITAL_DFP_1_X) { info->bpc = 8; DRM_DEBUG("%s: Assigning DFP sink color depth as %d bpc.\n", connector->name, info->bpc); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm: Add a debug print for drm_modeset_backoff()
From: Ville Syrjälä Logs can get confusing when some operations are done multiple times due to the ww mutex backoff. Add a debug print into drm_modeset_backoff() so that at least the reason for the odd looking logs will be obvious. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modeset_lock.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 81dd11901ffd..1277ff18d993 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -295,6 +295,8 @@ int drm_modeset_backoff(struct drm_modeset_acquire_ctx *ctx) { struct drm_modeset_lock *contended = ctx->contended; + DRM_DEBUG_KMS("Retrying to avoid deadlock\n"); + ctx->contended = NULL; if (WARN_ON(!contended)) -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm: Add debug prints for the various object lookup errors
From: Ville Syrjälä Only some of the drm mode object lookups have a corresponding debug print for the lookup failure. That makes logs a bit hard to parse when you can't see where the bad object ID is being used. Add a bunch more debug prints, and unify their appearance. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic_uapi.c | 5 + drivers/gpu/drm/drm_color_mgmt.c | 8 ++-- drivers/gpu/drm/drm_connector.c | 5 - drivers/gpu/drm/drm_crtc.c| 12 +++- drivers/gpu/drm/drm_encoder.c | 4 +++- drivers/gpu/drm/drm_framebuffer.c | 4 +++- drivers/gpu/drm/drm_mode_object.c | 17 ++--- drivers/gpu/drm/drm_plane.c | 13 + drivers/gpu/drm/drm_property.c| 12 +--- drivers/gpu/drm/drm_vblank.c | 8 ++-- 10 files changed, 66 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 9a1f41adfc67..06390307e5a3 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1321,11 +1321,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, obj = drm_mode_object_find(dev, file_priv, obj_id, DRM_MODE_OBJECT_ANY); if (!obj) { + DRM_DEBUG_ATOMIC("Unknown object ID %d\n", obj_id); ret = -ENOENT; goto out; } if (!obj->properties) { + DRM_DEBUG_ATOMIC("Object ID %d has no properties\n", +obj_id); drm_mode_object_put(obj); ret = -ENOENT; goto out; @@ -1352,6 +1355,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, prop = drm_mode_obj_find_prop_id(obj, prop_id); if (!prop) { + DRM_DEBUG_ATOMIC("Unknown property ID %d\n", +prop_id); drm_mode_object_put(obj); ret = -ENOENT; goto out; diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 07dcf47daafe..a99ee15b8328 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -245,8 +245,10 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev, return -EOPNOTSUPP; crtc = drm_crtc_find(dev, file_priv, crtc_lut->crtc_id); - if (!crtc) + if (!crtc) { + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_lut->crtc_id); return -ENOENT; + } if (crtc->funcs->gamma_set == NULL) return -ENOSYS; @@ -313,8 +315,10 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev, return -EOPNOTSUPP; crtc = drm_crtc_find(dev, file_priv, crtc_lut->crtc_id); - if (!crtc) + if (!crtc) { + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_lut->crtc_id); return -ENOENT; + } /* memcpy into gamma store */ if (crtc_lut->gamma_size != crtc->gamma_size) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 847539645558..8745eb132fd4 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1952,8 +1952,11 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo)); connector = drm_connector_lookup(dev, file_priv, out_resp->connector_id); - if (!connector) + if (!connector) { + DRM_DEBUG_KMS("Unknown connector ID %d\n", + out_resp->connector_id); return -ENOENT; + } drm_connector_for_each_possible_encoder(connector, encoder, i) encoders_count++; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 7dabbaf033a1..e5f234ffcd23 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -369,8 +369,10 @@ int drm_mode_getcrtc(struct drm_device *dev, return -EOPNOTSUPP; crtc = drm_crtc_find(dev, file_priv, crtc_resp->crtc_id); - if (!crtc) + if (!crtc) { + DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_resp->crtc_id); return -ENOENT; + } plane = crtc->primary; @@ -586,8 +588,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, } else { fb = drm_framebuffer_lookup(dev, file_priv, crtc_req->fb_id); if (!fb) { - DRM_DEBUG_KMS("Unknown FB ID%d\n", - crtc_req->fb_id); + DRM_DEBUG_KMS("Unknown FB ID %d\n", + crtc_req->fb_id);
[PATCH 2/3] drm: Sync errno values for property lookup errors
From: Ville Syrjälä Use ENOENT consistently for the case where the requested property isn't found, and EINVAL for the case where the object has no properties whatsoever. Currenrly these are handled differently in the atomic and legacy codepaths. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic_uapi.c | 2 +- drivers/gpu/drm/drm_mode_object.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 06390307e5a3..2a54f826cf65 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1330,7 +1330,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, DRM_DEBUG_ATOMIC("Object ID %d has no properties\n", obj_id); drm_mode_object_put(obj); - ret = -ENOENT; + ret = -EINVAL; goto out; } diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index e8dac94d576d..31730d935842 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -527,6 +527,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, property = drm_mode_obj_find_prop_id(arg_obj, arg->prop_id); if (!property) { DRM_DEBUG_KMS("Unknown property ID %d\n", arg->prop_id); + ret = -ENOENT; goto out_unref; } -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm/dp/mst: Provide better debugs for NAK replies
From: Ville Syrjälä Decode the NAK reply fields to make it easier to parse the logs. v2: s/STR/DP_STR/ to avoid conflict with some header stuff (0day) Use drm_dp_mst_req_type_str() more (DK) Signed-off-by: Ville Syrjälä Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/drm_dp_mst_topology.c | 71 +-- include/drm/drm_dp_helper.h | 1 + 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index c2ae56eabe44..0eafde53bd81 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -67,6 +67,64 @@ static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr, static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux); static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux); static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr); + +#define DP_STR(x) [DP_ ## x] = #x + +static const char *drm_dp_mst_req_type_str(u8 req_type) +{ + static const char * const req_type_str[] = { + DP_STR(GET_MSG_TRANSACTION_VERSION), + DP_STR(LINK_ADDRESS), + DP_STR(CONNECTION_STATUS_NOTIFY), + DP_STR(ENUM_PATH_RESOURCES), + DP_STR(ALLOCATE_PAYLOAD), + DP_STR(QUERY_PAYLOAD), + DP_STR(RESOURCE_STATUS_NOTIFY), + DP_STR(CLEAR_PAYLOAD_ID_TABLE), + DP_STR(REMOTE_DPCD_READ), + DP_STR(REMOTE_DPCD_WRITE), + DP_STR(REMOTE_I2C_READ), + DP_STR(REMOTE_I2C_WRITE), + DP_STR(POWER_UP_PHY), + DP_STR(POWER_DOWN_PHY), + DP_STR(SINK_EVENT_NOTIFY), + DP_STR(QUERY_STREAM_ENC_STATUS), + }; + + if (req_type >= ARRAY_SIZE(req_type_str) || + !req_type_str[req_type]) + return "unknown"; + + return req_type_str[req_type]; +} + +#undef DP_STR +#define DP_STR(x) [DP_NAK_ ## x] = #x + +static const char *drm_dp_mst_nak_reason_str(u8 nak_reason) +{ + static const char * const nak_reason_str[] = { + DP_STR(WRITE_FAILURE), + DP_STR(INVALID_READ), + DP_STR(CRC_FAILURE), + DP_STR(BAD_PARAM), + DP_STR(DEFER), + DP_STR(LINK_FAILURE), + DP_STR(NO_RESOURCES), + DP_STR(DPCD_FAIL), + DP_STR(I2C_NAK), + DP_STR(ALLOCATE_FAIL), + }; + + if (nak_reason >= ARRAY_SIZE(nak_reason_str) || + !nak_reason_str[nak_reason]) + return "unknown"; + + return nak_reason_str[nak_reason]; +} + +#undef DP_STR + /* sideband msg handling */ static u8 drm_dp_msg_header_crc4(const uint8_t *data, size_t num_nibbles) { @@ -594,7 +652,8 @@ static bool drm_dp_sideband_parse_reply(struct drm_dp_sideband_msg_rx *raw, case DP_POWER_UP_PHY: return drm_dp_sideband_parse_power_updown_phy_ack(raw, msg); default: - DRM_ERROR("Got unknown reply 0x%02x\n", msg->req_type); + DRM_ERROR("Got unknown reply 0x%02x (%s)\n", msg->req_type, + drm_dp_mst_req_type_str(msg->req_type)); return false; } } @@ -661,7 +720,8 @@ static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw, case DP_RESOURCE_STATUS_NOTIFY: return drm_dp_sideband_parse_resource_status_notify(raw, msg); default: - DRM_ERROR("Got unknown request 0x%02x\n", msg->req_type); + DRM_ERROR("Got unknown request 0x%02x (%s)\n", msg->req_type, + drm_dp_mst_req_type_str(msg->req_type)); return false; } } @@ -2747,7 +2807,12 @@ static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr) drm_dp_sideband_parse_reply(&mgr->down_rep_recv, &txmsg->reply); if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) - DRM_DEBUG_KMS("Got NAK reply: req 0x%02x, reason 0x%02x, nak data 0x%02x\n", txmsg->reply.req_type, txmsg->reply.u.nak.reason, txmsg->reply.u.nak.nak_data); + DRM_DEBUG_KMS("Got NAK reply: req 0x%02x (%s), reason 0x%02x (%s), nak data 0x%02x\n", + txmsg->reply.req_type, + drm_dp_mst_req_type_str(txmsg->reply.req_type), + txmsg->reply.u.nak.reason, + drm_dp_mst_nak_reason_str(txmsg->reply.u.nak.reason), + txmsg->reply.u.nak.nak_data); memset(&mgr->down_rep_recv, 0, sizeof(struct drm_dp_sideband_msg_rx)); drm_dp_mst_topology_put_mstb(mstb); diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index a62d3750663a..5db7fb8c8b50 100644 --- a/includ
[PATCH v2 1/2] drm/dp/mst: Provide defines for ACK vs. NAK reply type
From: Ville Syrjälä Make the code a bit easier to read by providing symbolic names for the reply_type (ACK vs. NAK). Also clean up some brace stuff while at it. v2: s/DP_REPLY/DP_SIDEBAND_REPLY/ (DK) Fix some checkpatch issues Signed-off-by: Ville Syrjälä Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/drm_dp_mst_topology.c | 26 +- include/drm/drm_dp_helper.h | 4 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 196ebba8af5f..c2ae56eabe44 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -568,7 +568,7 @@ static bool drm_dp_sideband_parse_reply(struct drm_dp_sideband_msg_rx *raw, msg->reply_type = (raw->msg[0] & 0x80) >> 7; msg->req_type = (raw->msg[0] & 0x7f); - if (msg->reply_type) { + if (msg->reply_type == DP_SIDEBAND_REPLY_NAK) { memcpy(msg->u.nak.guid, &raw->msg[1], 16); msg->u.nak.reason = raw->msg[17]; msg->u.nak.nak_data = raw->msg[18]; @@ -1969,9 +1969,9 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr, if (ret > 0) { int i; - if (txmsg->reply.reply_type == 1) + if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) { DRM_DEBUG_KMS("link address nak received\n"); - else { + } else { DRM_DEBUG_KMS("link address reply: %d\n", txmsg->reply.u.link_addr.nports); for (i = 0; i < txmsg->reply.u.link_addr.nports; i++) { DRM_DEBUG_KMS("port %d: input %d, pdt: %d, pn: %d, dpcd_rev: %02x, mcs: %d, ddps: %d, ldps %d, sdp %d/%d\n", i, @@ -2020,9 +2020,9 @@ static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr, ret = drm_dp_mst_wait_tx_reply(mstb, txmsg); if (ret > 0) { - if (txmsg->reply.reply_type == 1) + if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) { DRM_DEBUG_KMS("enum path resources nak received\n"); - else { + } else { if (port->port_num != txmsg->reply.u.path_resources.port_number) DRM_ERROR("got incorrect port in response\n"); DRM_DEBUG_KMS("enum path resources %d: %d %d\n", txmsg->reply.u.path_resources.port_number, txmsg->reply.u.path_resources.full_payload_bw_number, @@ -2132,7 +2132,7 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr, */ ret = drm_dp_mst_wait_tx_reply(mstb, txmsg); if (ret > 0) { - if (txmsg->reply.reply_type == 1) + if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) ret = -EINVAL; else ret = 0; @@ -2165,7 +2165,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr, ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg); if (ret > 0) { - if (txmsg->reply.reply_type == 1) + if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) ret = -EINVAL; else ret = 0; @@ -2423,9 +2423,9 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr, ret = drm_dp_mst_wait_tx_reply(mstb, txmsg); if (ret > 0) { - if (txmsg->reply.reply_type == 1) { + if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) ret = -EINVAL; - } else + else ret = 0; } kfree(txmsg); @@ -2438,7 +2438,7 @@ static int drm_dp_encode_up_ack_reply(struct drm_dp_sideband_msg_tx *msg, u8 req { struct drm_dp_sideband_msg_reply_body reply; - reply.reply_type = 0; + reply.reply_type = DP_SIDEBAND_REPLY_ACK; reply.req_type = req_type; drm_dp_encode_sideband_reply(&reply, msg); return 0; @@ -2745,9 +2745,9 @@ static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr) } drm_dp_sideband_parse_reply(&mgr->down_rep_recv, &txmsg->reply); - if (txmsg->reply.reply_type == 1) { + + if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) DRM_DEBUG_KMS("Got NAK reply: req 0x%02x, reason 0x%02x, nak data 0x%02x\n", txmsg->reply.req_type, txmsg->reply.u.nak.reason, txmsg->reply.u.nak.nak_data); - } memset(&mgr->down_rep_recv, 0, sizeof(struct drm_dp_sideband_msg_rx)); drm_dp_mst_topology_put_mstb(mstb); @@ -3893,7 +3893,7 @@ static int drm_dp_mst_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs ret = drm_dp_mst_wait_t
[PATCH] drm: Constify drm_color_lut_check()
From: Ville Syrjälä drm_color_lut_check() doens't modify the passed in blob so let's make it const. Also s/uint32_y/u32/ while at it. Cc: Matt Roper Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_color_mgmt.c | 6 +++--- include/drm/drm_color_mgmt.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 968ca7c91ad8..3c8826a91a03 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -474,10 +474,10 @@ EXPORT_SYMBOL(drm_plane_create_color_properties); * * Returns 0 on success, -EINVAL on failure. */ -int drm_color_lut_check(struct drm_property_blob *lut, - uint32_t tests) +int drm_color_lut_check(const struct drm_property_blob *lut, + u32 tests) { - struct drm_color_lut *entry; + const struct drm_color_lut *entry; int i; if (!lut || !tests) diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index 6affbda6d9cb..58d4b20b5b97 100644 --- a/include/drm/drm_color_mgmt.h +++ b/include/drm/drm_color_mgmt.h @@ -96,6 +96,6 @@ enum drm_color_lut_tests { DRM_COLOR_LUT_NON_DECREASING = BIT(1), }; -int drm_color_lut_check(struct drm_property_blob *lut, - uint32_t tests); +int drm_color_lut_check(const struct drm_property_blob *lut, + u32 tests); #endif -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Nuke drm_calc_{h,v}scale_relaxed()
From: Ville Syrjälä The fuzzy drm_calc_{h,v}scale_relaxed() helpers are no longer used. Throw them in the bin. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_rect.c | 108 - include/drm/drm_rect.h | 6 --- 2 files changed, 114 deletions(-) diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c index 8c057829b804..66c41b12719c 100644 --- a/drivers/gpu/drm/drm_rect.c +++ b/drivers/gpu/drm/drm_rect.c @@ -207,114 +207,6 @@ int drm_rect_calc_vscale(const struct drm_rect *src, } EXPORT_SYMBOL(drm_rect_calc_vscale); -/** - * drm_calc_hscale_relaxed - calculate the horizontal scaling factor - * @src: source window rectangle - * @dst: destination window rectangle - * @min_hscale: minimum allowed horizontal scaling factor - * @max_hscale: maximum allowed horizontal scaling factor - * - * Calculate the horizontal scaling factor as - * (@src width) / (@dst width). - * - * If the calculated scaling factor is below @min_vscale, - * decrease the height of rectangle @dst to compensate. - * - * If the calculated scaling factor is above @max_vscale, - * decrease the height of rectangle @src to compensate. - * - * If the scale is below 1 << 16, round down. If the scale is above - * 1 << 16, round up. This will calculate the scale with the most - * pessimistic limit calculation. - * - * RETURNS: - * The horizontal scaling factor. - */ -int drm_rect_calc_hscale_relaxed(struct drm_rect *src, -struct drm_rect *dst, -int min_hscale, int max_hscale) -{ - int src_w = drm_rect_width(src); - int dst_w = drm_rect_width(dst); - int hscale = drm_calc_scale(src_w, dst_w); - - if (hscale < 0 || dst_w == 0) - return hscale; - - if (hscale < min_hscale) { - int max_dst_w = src_w / min_hscale; - - drm_rect_adjust_size(dst, max_dst_w - dst_w, 0); - - return min_hscale; - } - - if (hscale > max_hscale) { - int max_src_w = dst_w * max_hscale; - - drm_rect_adjust_size(src, max_src_w - src_w, 0); - - return max_hscale; - } - - return hscale; -} -EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed); - -/** - * drm_rect_calc_vscale_relaxed - calculate the vertical scaling factor - * @src: source window rectangle - * @dst: destination window rectangle - * @min_vscale: minimum allowed vertical scaling factor - * @max_vscale: maximum allowed vertical scaling factor - * - * Calculate the vertical scaling factor as - * (@src height) / (@dst height). - * - * If the calculated scaling factor is below @min_vscale, - * decrease the height of rectangle @dst to compensate. - * - * If the calculated scaling factor is above @max_vscale, - * decrease the height of rectangle @src to compensate. - * - * If the scale is below 1 << 16, round down. If the scale is above - * 1 << 16, round up. This will calculate the scale with the most - * pessimistic limit calculation. - * - * RETURNS: - * The vertical scaling factor. - */ -int drm_rect_calc_vscale_relaxed(struct drm_rect *src, -struct drm_rect *dst, -int min_vscale, int max_vscale) -{ - int src_h = drm_rect_height(src); - int dst_h = drm_rect_height(dst); - int vscale = drm_calc_scale(src_h, dst_h); - - if (vscale < 0 || dst_h == 0) - return vscale; - - if (vscale < min_vscale) { - int max_dst_h = src_h / min_vscale; - - drm_rect_adjust_size(dst, 0, max_dst_h - dst_h); - - return min_vscale; - } - - if (vscale > max_vscale) { - int max_src_h = dst_h * max_vscale; - - drm_rect_adjust_size(src, 0, max_src_h - src_h); - - return max_vscale; - } - - return vscale; -} -EXPORT_SYMBOL(drm_rect_calc_vscale_relaxed); - /** * drm_rect_debug_print - print the rectangle information * @prefix: prefix string diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h index 6c54544a4be7..6195820aa5c5 100644 --- a/include/drm/drm_rect.h +++ b/include/drm/drm_rect.h @@ -182,12 +182,6 @@ int drm_rect_calc_hscale(const struct drm_rect *src, int drm_rect_calc_vscale(const struct drm_rect *src, const struct drm_rect *dst, int min_vscale, int max_vscale); -int drm_rect_calc_hscale_relaxed(struct drm_rect *src, -struct drm_rect *dst, -int min_hscale, int max_hscale); -int drm_rect_calc_vscale_relaxed(struct drm_rect *src, -struct drm_rect *dst, -int min_vscale, int max_vscale); void drm_rect_debug_print(const char *prefix, const struct drm_rect *r, bool fixed_point); void drm_rect_rotate(struct drm_rect *r, -- 2.19.2
[PATCH 1/4] drm/edid: Pass connector to AVI infoframe functions
From: Ville Syrjälä Make life easier for drivers by simply passing the connector to drm_hdmi_avi_infoframe_from_display_mode() and drm_hdmi_avi_infoframe_quant_range(). That way drivers don't need to worry about is_hdmi2_sink mess. v2: Make is_hdmi2_sink() return true for sil-sii8620 Adapt to omap/vc4 changes Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: Archit Taneja Cc: Andrzej Hajda Cc: Laurent Pinchart Cc: Inki Dae Cc: Joonyoung Shim Cc: Seung-Woo Kim Cc: Kyungmin Park Cc: Russell King Cc: CK Hu Cc: Philipp Zabel Cc: Rob Clark Cc: Ben Skeggs Cc: Tomi Valkeinen Cc: Sandy Huang Cc: "Heiko Stübner" Cc: Benjamin Gaignard Cc: Vincent Abriou Cc: Thierry Reding Cc: Eric Anholt Cc: Shawn Guo Cc: Ilia Mirkin Cc: amd-...@lists.freedesktop.org Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: linux-te...@vger.kernel.org Signed-off-by: Ville Syrjälä Acked-by: Thierry Reding Acked-by: Russell King Reviewed-by: Laurent Pinchart Reviewed-by: Jani Nikula --- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 3 ++- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 ++-- drivers/gpu/drm/bridge/sii902x.c | 3 ++- drivers/gpu/drm/bridge/sil-sii8620.c | 3 +-- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 ++- drivers/gpu/drm/drm_edid.c| 33 ++- drivers/gpu/drm/exynos/exynos_hdmi.c | 3 ++- drivers/gpu/drm/i2c/tda998x_drv.c | 3 ++- drivers/gpu/drm/i915/intel_hdmi.c | 14 +- drivers/gpu/drm/i915/intel_lspcon.c | 15 ++- drivers/gpu/drm/i915/intel_sdvo.c | 10 --- drivers/gpu/drm/mediatek/mtk_hdmi.c | 3 ++- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c| 3 ++- drivers/gpu/drm/nouveau/dispnv50/disp.c | 7 +++-- drivers/gpu/drm/omapdrm/omap_encoder.c| 4 +-- drivers/gpu/drm/radeon/radeon_audio.c | 2 +- drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++- drivers/gpu/drm/sti/sti_hdmi.c| 3 ++- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c| 3 ++- drivers/gpu/drm/tegra/hdmi.c | 3 ++- drivers/gpu/drm/tegra/sor.c | 3 ++- drivers/gpu/drm/vc4/vc4_hdmi.c| 9 --- drivers/gpu/drm/zte/zx_hdmi.c | 4 ++- include/drm/drm_edid.h| 8 +++--- 27 files changed, 91 insertions(+), 66 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index 4cfecdce29a3..1f0426d2fc2a 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -1682,7 +1682,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder, dce_v10_0_audio_write_sad_regs(encoder); dce_v10_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c index 7c868916d90f..2280b971d758 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c @@ -1724,7 +1724,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder, dce_v11_0_audio_write_sad_regs(encoder); dce_v11_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c index 17eaaba36017..db443ec53d3a 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c @@ -1423,6 +1423,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder, struct amdgpu_device *adev = dev->dev_private; struct amdgpu_encoder *amdgpu_encoder = to_amdgpu_encoder(encoder); struct amdgpu_encoder_atom_dig *dig = amdgpu_encoder->enc_priv; + struct drm_connector *connector = amdgpu_get_connector_for_encoder(encoder); struct hdmi_avi_infoframe frame; u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE]; uint8_t *payload = buffer + 3; @@ -1430,7 +1431,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder, ssize_t err; u32 tmp; - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_
[PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well
From: Ville Syrjälä Fill out the AVI infoframe quantization range bits using drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as well. This changes the behaviour slightly as drm_hdmi_avi_infoframe_quant_range() will set a non-zero Q bit even when QS==0 iff the Q bit matched the default quantization range for the given mode. This matches the recommendation in HDMI 2.0 and is allowed even before that. v2: Pimp commit msg (DK) Signed-off-by: Ville Syrjälä Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/intel_sdvo.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 1277d31adb54..9c16e273fb8d 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -984,6 +984,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, const struct intel_crtc_state *pipe_config, const struct drm_connector_state *conn_state) { + const struct drm_display_mode *adjusted_mode = + &pipe_config->base.adjusted_mode; uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)]; union hdmi_infoframe frame; int ret; @@ -991,20 +993,19 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, conn_state->connector, - &pipe_config->base.adjusted_mode); + adjusted_mode); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return false; } - if (intel_sdvo->rgb_quant_range_selectable) { - if (pipe_config->limited_color_range) - frame.avi.quantization_range = - HDMI_QUANTIZATION_RANGE_LIMITED; - else - frame.avi.quantization_range = - HDMI_QUANTIZATION_RANGE_FULL; - } + drm_hdmi_avi_infoframe_quant_range(&frame.avi, + conn_state->connector, + adjusted_mode, + pipe_config->limited_color_range ? + HDMI_QUANTIZATION_RANGE_LIMITED : + HDMI_QUANTIZATION_RANGE_FULL, + intel_sdvo->rgb_quant_range_selectable); len = hdmi_infoframe_pack(&frame, sdvo_data, sizeof(sdvo_data)); if (len < 0) -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/edid: Add display_info.rgb_quant_range_selectable
From: Ville Syrjälä Move the CEA-861 QS bit handling entirely into the edid code. No need to bother the drivers with this. Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: amd-...@lists.freedesktop.org Cc: Eric Anholt (supporter:DRM DRIVERS FOR VC4) Signed-off-by: Ville Syrjälä Acked-by: Alex Deucher Acked-by: Eric Anholt --- drivers/gpu/drm/drm_edid.c| 70 --- drivers/gpu/drm/i915/intel_drv.h | 1 - drivers/gpu/drm/i915/intel_hdmi.c | 8 +-- drivers/gpu/drm/i915/intel_lspcon.c | 3 +- drivers/gpu/drm/i915/intel_sdvo.c | 7 +-- drivers/gpu/drm/radeon/radeon_audio.c | 3 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 9 +--- include/drm/drm_connector.h | 6 +++ include/drm/drm_edid.h| 4 +- 9 files changed, 43 insertions(+), 68 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index cd25bd08bf53..990b1909f9d7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3641,6 +3641,20 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) return oui == HDMI_FORUM_IEEE_OUI; } +static bool cea_db_is_vcdb(const u8 *db) +{ + if (cea_db_tag(db) != USE_EXTENDED_TAG) + return false; + + if (cea_db_payload_len(db) != 2) + return false; + + if (cea_db_extended_tag(db) != EXT_VIDEO_CAPABILITY_BLOCK) + return false; + + return true; +} + static bool cea_db_is_y420cmdb(const u8 *db) { if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4223,41 +4237,6 @@ bool drm_detect_monitor_audio(struct edid *edid) } EXPORT_SYMBOL(drm_detect_monitor_audio); -/** - * drm_rgb_quant_range_selectable - is RGB quantization range selectable? - * @edid: EDID block to scan - * - * Check whether the monitor reports the RGB quantization range selection - * as supported. The AVI infoframe can then be used to inform the monitor - * which quantization range (full or limited) is used. - * - * Return: True if the RGB quantization range is selectable, false otherwise. - */ -bool drm_rgb_quant_range_selectable(struct edid *edid) -{ - u8 *edid_ext; - int i, start, end; - - edid_ext = drm_find_cea_extension(edid); - if (!edid_ext) - return false; - - if (cea_db_offsets(edid_ext, &start, &end)) - return false; - - for_each_cea_db(edid_ext, i, start, end) { - if (cea_db_tag(&edid_ext[i]) == USE_EXTENDED_TAG && - cea_db_payload_len(&edid_ext[i]) == 2 && - cea_db_extended_tag(&edid_ext[i]) == - EXT_VIDEO_CAPABILITY_BLOCK) { - DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", edid_ext[i + 2]); - return edid_ext[i + 2] & EDID_CEA_VCDB_QS; - } - } - - return false; -} -EXPORT_SYMBOL(drm_rgb_quant_range_selectable); /** * drm_default_rgb_quant_range - default RGB quantization range @@ -4278,6 +4257,16 @@ drm_default_rgb_quant_range(const struct drm_display_mode *mode) } EXPORT_SYMBOL(drm_default_rgb_quant_range); +static void drm_parse_vcdb(struct drm_connector *connector, const u8 *db) +{ + struct drm_display_info *info = &connector->display_info; + + DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", db[2]); + + if (db[2] & EDID_CEA_VCDB_QS) + info->rgb_quant_range_selectable = true; +} + static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector, const u8 *db) { @@ -4452,6 +4441,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, drm_parse_hdmi_forum_vsdb(connector, db); if (cea_db_is_y420cmdb(db)) drm_parse_y420cmdb_bitmap(connector, db); + if (cea_db_is_vcdb(db)) + drm_parse_vcdb(connector, db); } } @@ -4472,6 +4463,7 @@ drm_reset_display_info(struct drm_connector *connector) info->max_tmds_clock = 0; info->dvi_dual = false; info->has_hdmi_infoframe = false; + info->rgb_quant_range_selectable = false; memset(&info->hdmi, 0, sizeof(info->hdmi)); info->non_desktop = 0; @@ -4939,15 +4931,15 @@ EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); * @connector: the connector * @mode: DRM display mode * @rgb_quant_range: RGB quantization range (Q) - * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS) */ void drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, struct drm_connector *connector, const struct drm_display_mode *mode, - enum hdmi_quantization_range rgb_quant_range, - bool rgb_quant_range_selectable) + enum hdmi_quantization_ran
[PATCH 3/4] drm/radeon: Use drm_hdmi_avi_infoframe_quant_range()
From: Ville Syrjälä Fill out the AVI infoframe quantization range bits using drm_hdmi_avi_infoframe_quant_range() instead of hand rolling it. This changes the behaviour slightly as drm_hdmi_avi_infoframe_quant_range() will set a non-zero Q bit even when QS==0 iff the Q bit matched the default quantization range for the given mode. This matches the recommendation in HDMI 2.0 and is allowed even before that. Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: amd-...@lists.freedesktop.org Signed-off-by: Ville Syrjälä Acked-by: Alex Deucher --- drivers/gpu/drm/radeon/radeon_audio.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c index 5a7d48339b32..708765bf9e66 100644 --- a/drivers/gpu/drm/radeon/radeon_audio.c +++ b/drivers/gpu/drm/radeon/radeon_audio.c @@ -523,14 +523,11 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder, } if (radeon_encoder->output_csc != RADEON_OUTPUT_CSC_BYPASS) { - if (drm_rgb_quant_range_selectable(radeon_connector_edid(connector))) { - if (radeon_encoder->output_csc == RADEON_OUTPUT_CSC_TVRGB) - frame.quantization_range = HDMI_QUANTIZATION_RANGE_LIMITED; - else - frame.quantization_range = HDMI_QUANTIZATION_RANGE_FULL; - } else { - frame.quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT; - } + drm_hdmi_avi_infoframe_quant_range(&frame, connector, mode, + radeon_encoder->output_csc == RADEON_OUTPUT_CSC_TVRGB ? + HDMI_QUANTIZATION_RANGE_LIMITED : + HDMI_QUANTIZATION_RANGE_FULL, + drm_rgb_quant_range_selectable(radeon_connector_edid(connector))); } err = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer)); -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/edid: If no preferred mode is found assume the first mode is preferred
From: Ville Syrjälä Some monitors apparently forget to mark any mode as preferred in the EDID. In this particular case we have a very generic looking ID "PNP Model 0 Serial Number 4" / "LVDS 800x600" so a specific quirk doesn't seem particularly wise. Also the quirk we have (EDID_QUIRK_FIRST_DETAILED_PREFERRED) is actually defunct so we'd have to fix it first. As a generic fallback let's just mark the first probed mode (which should be the first detailed mode, assuming there are any) as preferred. On this particular machine the VBIOS seems to pick the 800x600 60Hz EST mode, which has the opposite sync polarities to the 800x600 60Hz detailed timings. But since it works I guess we can ignore that slight discrepancy. Cc: Adam Jackson Cc: Roberto Viola Tested-by: Roberto Viola Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109780 Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_edid.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 5f142530532a..6c6a93647686 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1828,7 +1828,7 @@ static void edid_fixup_preferred(struct drm_connector *connector, list_for_each_entry_safe(cur_mode, t, &connector->probed_modes, head) { cur_mode->type &= ~DRM_MODE_TYPE_PREFERRED; - if (cur_mode == preferred_mode) + if (cur_mode == preferred_mode || target_refresh == 0) continue; /* Largest mode is preferred */ @@ -1850,6 +1850,18 @@ static void edid_fixup_preferred(struct drm_connector *connector, preferred_mode->type |= DRM_MODE_TYPE_PREFERRED; } +static bool preferred_mode_exists(struct drm_connector *connector) +{ + struct drm_display_mode *mode; + + list_for_each_entry(mode, &connector->probed_modes, head) { + if (mode->type & DRM_MODE_TYPE_PREFERRED) + return true; + } + + return false; +} + static bool mode_is_rb(const struct drm_display_mode *mode) { @@ -4733,7 +4745,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) num_modes += add_inferred_modes(connector, edid); - if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75)) + if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75) || + !preferred_mode_exists(connector)) edid_fixup_preferred(connector, quirks); if (quirks & EDID_QUIRK_FORCE_6BPC) -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/edid: Remove defunct EDID_QUIRK_FIRST_DETAILED_PREFERRED
From: Ville Syrjälä Looks like EDID_QUIRK_FIRST_DETAILED_PREFERRED never did anything. Its counterpart in f86EdidModes.c is properly hooked up but somehow that functionality was lost when it was copied into the kernel. Assuming that another preferred mode didn't sneak in somehow (is that even possible?) this should now be fully covered by the generic fallback that marks the first probed mode as preferred. So let's just remove the quirk. I tried to double check the known cases: * Proview AY765C https://bugs.freedesktop.org/show_bug.cgi?id=15160 seems OK * Unknown Acer https://bugzilla.redhat.com/show_bug.cgi?id=284231 (got the reference from xf86EdidModes.c), no access * Philips 107p5 CRT "Reported on xorg@ with pastebin", didn't find the mail(s) * Peacock Ergovision 19 (only in xf86EdidModes.c) https://bugzilla.redhat.com/show_bug.cgi?id=492359 seems OK Cc: Adam Jackson Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_edid.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6c6a93647686..59829e596910 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -68,8 +68,6 @@ * maximum size and use that. */ #define EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE (1 << 4) -/* Monitor forgot to set the first detailed is preferred bit. */ -#define EDID_QUIRK_FIRST_DETAILED_PREFERRED(1 << 5) /* use +hsync +vsync for detailed mode */ #define EDID_QUIRK_DETAILED_SYNC_PP(1 << 6) /* Force reduced-blanking timings for detailed modes */ @@ -107,8 +105,6 @@ static const struct edid_quirk { { "ACR", 44358, EDID_QUIRK_PREFER_LARGE_60 }, /* Acer F51 */ { "API", 0x7602, EDID_QUIRK_PREFER_LARGE_60 }, - /* Unknown Acer */ - { "ACR", 2423, EDID_QUIRK_FIRST_DETAILED_PREFERRED }, /* AEO model 0 reports 8 bpc, but is a 6 bpc panel */ { "AEO", 0, EDID_QUIRK_FORCE_6BPC }, @@ -145,12 +141,6 @@ static const struct edid_quirk { { "LPL", 0, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE }, { "LPL", 0x2a00, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE }, - /* Philips 107p5 CRT */ - { "PHL", 57364, EDID_QUIRK_FIRST_DETAILED_PREFERRED }, - - /* Proview AY765C */ - { "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED }, - /* Samsung SyncMaster 205BW. Note: irony */ { "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP }, /* Samsung SyncMaster 22[5-6]BW */ -- 2.19.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/fb-helper: Do not assume drm_mode_create_from_cmdline_mode() can't fail
From: Ville Syrjälä drm_mode_create_from_cmdline_mode() can return NULL, so the caller should check for that. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_fb_helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index b9b7c06cbc4f..bdfa14cd7f6d 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2205,7 +2205,9 @@ struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector *f create_mode: mode = drm_mode_create_from_cmdline_mode(fb_helper_conn->connector->dev, cmdline_mode); - list_add(&mode->head, &fb_helper_conn->connector->modes); + if (mode) + list_add(&mode->head, &fb_helper_conn->connector->modes); + return mode; } EXPORT_SYMBOL(drm_pick_cmdline_mode); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/i915: Throw away the BIOS fb if has the wrong depth/bpp
From: Ville Syrjälä Respect the user's choice of depth/bpp for the fbdev framebuffer and throw out the fb we inherited from the BIOS if it doesn't match. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_fbdev.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 0d3a6fa674e6..1a935dc74d23 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -199,6 +199,17 @@ static int intelfb_create(struct drm_fb_helper *helper, drm_framebuffer_put(&intel_fb->base); intel_fb = ifbdev->fb = NULL; } + if (intel_fb && + (sizes->surface_depth != intel_fb->base.format->depth || +sizes->surface_bpp != intel_fb->base.format->cpp[0] * 8)) { + DRM_DEBUG_KMS("BIOS fb using wrong depth/bpp (%d/%d), we require (%d/%d)," + " releasing it\n", + intel_fb->base.format->depth, + intel_fb->base.format->cpp[0] * 8, + sizes->surface_depth, sizes->surface_bpp); + drm_framebuffer_put(&intel_fb->base); + intel_fb = ifbdev->fb = NULL; + } if (!intel_fb || WARN_ON(!intel_fb_obj(&intel_fb->base))) { DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n"); ret = intelfb_alloc(helper, sizes); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/fb-helper: Set up gamma_lut during restore_fbdev_mode_atomic()
From: Ville Syrjälä i915 will now refuse to enable a C8 plane unless the gamma_lut is already enabled (to avoid scanning out whatever garbage got left in the hw LUT previously). So in order to make the fbdev code work with C8 we need to program the gamma_lut already during restore_fbdev_mode_atomic(). To avoid having to update the hw LUT every time restore_fbdev_mode_atomic() is called we hang on to the current gamma_lut blob. Note that the first crtc to get enabled will dictate the size of the gamma_lut, so this approach isn't 100% great for hardware with non-uniform crtcs. But that problem already exists in setcmap_atomic() so we'll just keep ignoring it. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_fb_helper.c | 165 include/drm/drm_fb_helper.h | 7 ++ 2 files changed, 113 insertions(+), 59 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index bdfa14cd7f6d..0ecec763789f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -436,10 +436,76 @@ static bool drm_fb_helper_panel_rotation(struct drm_mode_set *modeset, return true; } +static int setcmap_crtc_atomic(struct drm_atomic_state *state, + struct drm_crtc *crtc, + struct drm_property_blob *gamma_lut) +{ + struct drm_crtc_state *crtc_state; + bool replaced; + + crtc_state = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + + replaced = drm_property_replace_blob(&crtc_state->degamma_lut, + NULL); + replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL); + replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, + gamma_lut); + crtc_state->color_mgmt_changed |= replaced; + + return 0; +} + +static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc, + struct fb_cmap *cmap) +{ + struct drm_device *dev = crtc->dev; + struct drm_property_blob *gamma_lut; + struct drm_color_lut *lut; + int size = crtc->gamma_size; + int i; + + if (!size || cmap->start + cmap->len > size) + return ERR_PTR(-EINVAL); + + gamma_lut = drm_property_create_blob(dev, sizeof(*lut) * size, NULL); + if (IS_ERR(gamma_lut)) + return gamma_lut; + + lut = gamma_lut->data; + if (cmap->start || cmap->len != size) { + u16 *r = crtc->gamma_store; + u16 *g = r + crtc->gamma_size; + u16 *b = g + crtc->gamma_size; + + for (i = 0; i < cmap->start; i++) { + lut[i].red = r[i]; + lut[i].green = g[i]; + lut[i].blue = b[i]; + } + for (i = cmap->start + cmap->len; i < size; i++) { + lut[i].red = r[i]; + lut[i].green = g[i]; + lut[i].blue = b[i]; + } + } + + for (i = 0; i < cmap->len; i++) { + lut[cmap->start + i].red = cmap->red[i]; + lut[cmap->start + i].green = cmap->green[i]; + lut[cmap->start + i].blue = cmap->blue[i]; + } + + return gamma_lut; +} + static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool active) { + struct fb_info *info = fb_helper->fbdev; struct drm_client_dev *client = &fb_helper->client; struct drm_device *dev = fb_helper->dev; + struct drm_property_blob *gamma_lut; struct drm_plane_state *plane_state; struct drm_plane *plane; struct drm_atomic_state *state; @@ -455,6 +521,10 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ goto out_ctx; } + gamma_lut = fb_helper->gamma_lut; + if (gamma_lut) + drm_property_blob_get(gamma_lut); + state->acquire_ctx = &ctx; retry: drm_for_each_plane(plane, dev) { @@ -476,7 +546,8 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ } drm_client_for_each_modeset(mode_set, client) { - struct drm_plane *primary = mode_set->crtc->primary; + struct drm_crtc *crtc = mode_set->crtc; + struct drm_plane *primary = crtc->primary; unsigned int rotation; if (drm_fb_helper_panel_rotation(mode_set, &rotation)) { @@ -489,24 +560,46 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ if (ret != 0) goto out_state; + if (info->fix.visual != FB_VISUAL_TRUECOLOR) { + if (!gamma_lut) +
[PATCH 2/4] drm: Refuse to create zero width/height cmdline modes
From: Ville Syrjälä If the user specifies zero width/height cmdline mode i915 will blow up as the fbdev path will bypass the regular fb sanity check that would otherwise have refused to create a framebuffer with zero width/height. The reason I thought to try this is so that I can force a specific depth for fbdev without actually having to hardcode the mode on the kernel cmdline. Eg. if I pass video=0x0-8 I will get an 8bpp framebuffer at my monitor's native resolution. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modes.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 5a07a28fec6d..b36248a5d826 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1593,6 +1593,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev, { struct drm_display_mode *mode; + if (cmd->xres == 0 || cmd->yres == 0) + return NULL; + if (cmd->cvt) mode = drm_cvt_mode(dev, cmd->xres, cmd->yres, -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH 1/2] drm: Improve PATH prop docs
From: Ville Syrjälä The PATH blob is already being parsed by userspace for MST connectors so the layout of the blob is now uabi. Let's document what it should look like. Also add a clear note saying non-MST connectors can have a PATH prop too. Cc: Daniel Vetter Cc: Pekka Paalanen Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_connector.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index e17586aaa80f..ce3926e9ad11 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -899,7 +899,16 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = { * connected. Used by DP MST. This should be set by calling * drm_connector_set_path_property(), in the case of DP MST with the * path property the MST manager created. Userspace cannot change this - * property. + * property. The value must be an ASCII string. + * + * For DP MST connectors the path string follows the pattern + * "mst:[-]...", where the base connector ID + * identifies the DP connector on the source device, and the mst ports + * are the port numbers in the DP MST topology. + * + * For non-DP MST connectors the format is freeform, as long as it + * uniquely identifies the physical path, remains stable across + * kernel releases, and does not start with "mst:". * TILE: * Connector tile group property to indicate how a set of DRM connector * compose together into one logical screen. This is used by both high-res @@ -1678,7 +1687,7 @@ int drm_mode_create_suggested_offset_properties(struct drm_device *dev) EXPORT_SYMBOL(drm_mode_create_suggested_offset_properties); /** - * drm_connector_set_path_property - set tile property on connector + * drm_connector_set_path_property - set path property on connector * @connector: connector to set property on. * @path: path to use for property; must not be NULL. * -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH 0/2] drm: PATH prop for all connectors?
From: Ville Syrjälä Here's a possible apporoach for providing userspace with some stable connector identifiers. Combine with the bus name of the GPU and you should have some kind of real physical path description. Unfortunately the ship has sailed for MST connectors because userspace is already parsing the property and expects to find certain things there. So if we want stable names for those we'd probably have introduce another PATH prop (PHYS_PATH?). I suppose one alternative would to make the connector type_id stable. Currently that is being populated by drm core and it's just a global counter. Not sure how badly things would turn out if we'd allow each driver to set that. It could result in conflicting xrandr connector names between different GPUs which I suppose would confuse existing userspace? Cc: Daniel Vetter Cc: Pekka Paalanen Cc: Ilia Mirkin Ville Syrjälä (2): drm: Improve PATH prop docs drm/i915: Populate PATH prop for every connector drivers/gpu/drm/drm_connector.c| 13 -- drivers/gpu/drm/i915/icl_dsi.c | 3 +++ drivers/gpu/drm/i915/intel_connector.c | 20 +++ drivers/gpu/drm/i915/intel_connector.h | 3 +++ drivers/gpu/drm/i915/intel_crt.c | 2 ++ drivers/gpu/drm/i915/intel_dp.c| 6 - drivers/gpu/drm/i915/intel_dp_mst.c| 3 +-- drivers/gpu/drm/i915/intel_dvo.c | 3 +++ drivers/gpu/drm/i915/intel_hdmi.c | 4 +++ drivers/gpu/drm/i915/intel_lvds.c | 2 ++ drivers/gpu/drm/i915/intel_sdvo.c | 35 ++ drivers/gpu/drm/i915/intel_tv.c| 2 ++ drivers/gpu/drm/i915/vlv_dsi.c | 3 +++ 13 files changed, 83 insertions(+), 16 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH 2/2] drm/i915: Populate PATH prop for every connector
From: Ville Syrjälä Userspace may want stable identifiers for connectors. Let's try to provide that via the PATH prop. I tried to make these somewhat abstract by using just "port_type:index" type of approach, where we derive the index from the physical instance of that hw block, so it should remain stable even if we reorder things in the driver. Cc: Daniel Vetter Cc: Pekka Paalanen Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/icl_dsi.c | 3 +++ drivers/gpu/drm/i915/intel_connector.c | 20 +++ drivers/gpu/drm/i915/intel_connector.h | 3 +++ drivers/gpu/drm/i915/intel_crt.c | 2 ++ drivers/gpu/drm/i915/intel_dp.c| 6 - drivers/gpu/drm/i915/intel_dp_mst.c| 3 +-- drivers/gpu/drm/i915/intel_dvo.c | 3 +++ drivers/gpu/drm/i915/intel_hdmi.c | 4 +++ drivers/gpu/drm/i915/intel_lvds.c | 2 ++ drivers/gpu/drm/i915/intel_sdvo.c | 35 ++ drivers/gpu/drm/i915/intel_tv.c| 2 ++ drivers/gpu/drm/i915/vlv_dsi.c | 3 +++ 12 files changed, 72 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c index 74448e6bf749..54ccc69aa60a 100644 --- a/drivers/gpu/drm/i915/icl_dsi.c +++ b/drivers/gpu/drm/i915/icl_dsi.c @@ -1544,6 +1544,9 @@ void icl_dsi_init(struct drm_i915_private *dev_priv) /* attach connector to encoder */ intel_connector_attach_encoder(intel_connector, encoder); + intel_connector_set_path_property(connector, "dsi:%d", + port - PORT_A); + mutex_lock(&dev->mode_config.mutex); fixed_mode = intel_panel_vbt_fixed_mode(intel_connector); mutex_unlock(&dev->mode_config.mutex); diff --git a/drivers/gpu/drm/i915/intel_connector.c b/drivers/gpu/drm/i915/intel_connector.c index 073b6c3ab7cc..6c1b027fdb11 100644 --- a/drivers/gpu/drm/i915/intel_connector.c +++ b/drivers/gpu/drm/i915/intel_connector.c @@ -280,3 +280,23 @@ intel_attach_colorspace_property(struct drm_connector *connector) drm_object_attach_property(&connector->base, connector->colorspace_property, 0); } + +int intel_connector_set_path_property(struct drm_connector *connector, + const char *fmt, ...) +{ + char path[64]; + va_list ap; + int ret; + + va_start(ap, fmt); + ret = vsnprintf(path, sizeof(path), fmt, ap); + va_end(ap); + + if (WARN_ON(ret >= sizeof(path))) + return -EINVAL; + + drm_object_attach_property(&connector->base, + connector->dev->mode_config.path_property, 0); + + return drm_connector_set_path_property(connector, path); +} diff --git a/drivers/gpu/drm/i915/intel_connector.h b/drivers/gpu/drm/i915/intel_connector.h index 93a7375c8196..108777bc9545 100644 --- a/drivers/gpu/drm/i915/intel_connector.h +++ b/drivers/gpu/drm/i915/intel_connector.h @@ -31,5 +31,8 @@ void intel_attach_force_audio_property(struct drm_connector *connector); void intel_attach_broadcast_rgb_property(struct drm_connector *connector); void intel_attach_aspect_ratio_property(struct drm_connector *connector); void intel_attach_colorspace_property(struct drm_connector *connector); +__printf(2, 3) +int intel_connector_set_path_property(struct drm_connector *connector, + const char *fmt, ...); #endif /* __INTEL_CONNECTOR_H__ */ diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 3fcf2f84bcce..1383db646986 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -1048,6 +1048,8 @@ void intel_crt_init(struct drm_i915_private *dev_priv) if (!I915_HAS_HOTPLUG(dev_priv)) intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT; + intel_connector_set_path_property(connector, "crt:0"); + /* * Configure the automatic hotplug detection stuff */ diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4336df46fe78..c9071d25bd37 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6527,7 +6527,11 @@ static void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->dev); - enum port port = dp_to_dig_port(intel_dp)->base.port; + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; + enum port port = encoder->port; + + intel_connector_set_path_property(connector, "ddi:%d\n", + port - PORT_A); if (!IS_G4X(dev_priv) && port != PORT_A) intel_attach_force_audio_property(connector); diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 0caf645fbbb8..3bc0de2ff5af 1
[PATCH 0/5] drm: Aspect ratio fixes
From: Ville Syrjälä Ilia pointed out some oddball crap in the i915 aspect ratio handling. While looking at that I noticed a bunch of fail in the core as well. This series aims to fix it all. Cc: Ilia Mirkin Ville Syrjälä (5): drm: Do not use bitwise OR to set picure_aspect_ratio drm: Do not accept garbage mode aspect ratio flags drm: WARN on illegal aspect ratio when converting a mode to umode drm/i915: Do not override mode's aspect ratio with the prop value NONE drm/i915: Drop redundant aspec ratio prop value initialization drivers/gpu/drm/drm_modes.c | 17 +++-- drivers/gpu/drm/i915/display/intel_hdmi.c | 5 ++--- drivers/gpu/drm/i915/display/intel_sdvo.c | 4 +--- 3 files changed, 14 insertions(+), 12 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm: Do not use bitwise OR to set picure_aspect_ratio
From: Ville Syrjälä enum hdmi_picture_aspect is not a bitmask, so don't use bitwise OR to populate it. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modes.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 57e6408288c8..53acc6756ee0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1966,16 +1966,16 @@ int drm_mode_convert_umode(struct drm_device *dev, switch (in->flags & DRM_MODE_FLAG_PIC_AR_MASK) { case DRM_MODE_FLAG_PIC_AR_4_3: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3; break; case DRM_MODE_FLAG_PIC_AR_16_9: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9; break; case DRM_MODE_FLAG_PIC_AR_64_27: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_64_27; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27; break; case DRM_MODE_FLAG_PIC_AR_256_135: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_256_135; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135; break; default: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm: Do not accept garbage mode aspect ratio flags
From: Ville Syrjälä Don't let userspace feed us any old garbage in the mode aspect ratio flags. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modes.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 53acc6756ee0..847048dee048 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1977,9 +1977,11 @@ int drm_mode_convert_umode(struct drm_device *dev, case DRM_MODE_FLAG_PIC_AR_256_135: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135; break; - default: + case DRM_MODE_FLAG_PIC_AR_NONE: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; break; + default: + return -EINVAL; } out->status = drm_mode_validate_driver(dev, out); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm: WARN on illegal aspect ratio when converting a mode to umode
From: Ville Syrjälä WARN if the incoming drm_display_mode has an illegal aspect ratio when converting it to a user mode. This should never happen unless the driver made a mistake and put an invalid value into the aspect ratio. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modes.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 847048dee048..be2ccd8eccfd 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1906,8 +1906,11 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, case HDMI_PICTURE_ASPECT_256_135: out->flags |= DRM_MODE_FLAG_PIC_AR_256_135; break; - case HDMI_PICTURE_ASPECT_RESERVED: default: + WARN(1, "Invalid aspect ratio (0%x) on mode\n", +in->picture_aspect_ratio); + /* fall through */ + case HDMI_PICTURE_ASPECT_NONE: out->flags |= DRM_MODE_FLAG_PIC_AR_NONE; break; } -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/i915: Drop redundant aspec ratio prop value initialization
From: Ville Syrjälä HDMI_PICTURE_ASPECT_NONE is zero and the connector state is kzalloc()'d so no need to initialize conn_state->picture_aspect_ratio with it. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_hdmi.c | 1 - drivers/gpu/drm/i915/display/intel_sdvo.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 6a4650b44ac6..f95f3db5ecb8 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2809,7 +2809,6 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c intel_attach_colorspace_property(connector); drm_connector_attach_content_type_property(connector); - connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) drm_object_attach_property(&connector->base, diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 5cb619613157..c471fcce59f8 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -2624,7 +2624,6 @@ intel_sdvo_add_hdmi_properties(struct intel_sdvo *intel_sdvo, intel_attach_broadcast_rgb_property(&connector->base.base); } intel_attach_aspect_ratio_property(&connector->base.base); - connector->base.base.state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; } static struct intel_sdvo_connector *intel_sdvo_connector_alloc(void) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/i915: Do not override mode's aspect ratio with the prop value NONE
From: Ville Syrjälä HDMI_PICTURE_ASPECT_NONE means "Automatic" so when the user has that selected we should keep whatever aspect ratio the mode already has. Also no point in checking for connector->is_hdmi in the SDVO code since we only attach the property to HDMI connectors. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_hdmi.c | 5 +++-- drivers/gpu/drm/i915/display/intel_sdvo.c | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 0ebec69bbbfc..6a4650b44ac6 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2394,8 +2394,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, return -EINVAL; } - /* Set user selected PAR to incoming mode's member */ - adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio; + if (conn_state->picture_aspect_ratio) + adjusted_mode->picture_aspect_ratio = + conn_state->picture_aspect_ratio; pipe_config->lane_count = 4; diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index ceda03e5a3d4..5cb619613157 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -1321,9 +1321,9 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder, if (IS_TV(intel_sdvo_connector)) i9xx_adjust_sdvo_tv_clock(pipe_config); - /* Set user selected PAR to incoming mode's member */ - if (intel_sdvo_connector->is_hdmi) - adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio; + if (conn_state->picture_aspect_ratio) + adjusted_mode->picture_aspect_ratio = + conn_state->picture_aspect_ratio; if (!intel_sdvo_compute_avi_infoframe(intel_sdvo, pipe_config, conn_state)) { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Dump mode picture aspect ratio
From: Ville Syrjälä Currently the logs show nothing about the mode's aspect ratio. Include that information in the normal mode dump. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/video/hdmi.c| 3 ++- include/drm/drm_modes.h | 4 ++-- include/linux/hdmi.h| 3 +++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index b939bc28d886..bc593fe1c268 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -1057,7 +1057,7 @@ static const char *hdmi_colorimetry_get_name(enum hdmi_colorimetry colorimetry) return "Invalid"; } -static const char * +const char * hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect) { switch (picture_aspect) { @@ -1076,6 +1076,7 @@ hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect) } return "Invalid"; } +EXPORT_SYMBOL(hdmi_picture_aspect_get_name); static const char * hdmi_active_aspect_get_name(enum hdmi_active_aspect active_aspect) diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 083f16747369..2b1809c74fbe 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -431,7 +431,7 @@ struct drm_display_mode { /** * DRM_MODE_FMT - printf string for &struct drm_display_mode */ -#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x" +#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s" /** * DRM_MODE_ARG - printf arguments for &struct drm_display_mode @@ -441,7 +441,7 @@ struct drm_display_mode { (m)->name, (m)->vrefresh, (m)->clock, \ (m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \ (m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \ - (m)->type, (m)->flags + (m)->type, (m)->flags, hdmi_picture_aspect_get_name((m)->picture_aspect_ratio) #define obj_to_mode(x) container_of(x, struct drm_display_mode, base) diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index 9918a6c910c5..de7cbe385dba 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -434,4 +434,7 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, void hdmi_infoframe_log(const char *level, struct device *dev, const union hdmi_infoframe *frame); +const char * +hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect); + #endif /* _DRM_HDMI_H */ -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm: Dump mode picture aspect ratio
From: Ville Syrjälä Currently the logs show nothing about the mode's aspect ratio. Include that information in the normal mode dump. v2: Don't print anything for NONE (Ilia) Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/video/hdmi.c| 3 ++- include/drm/drm_modes.h | 6 -- include/linux/hdmi.h| 3 +++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index b939bc28d886..bc593fe1c268 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -1057,7 +1057,7 @@ static const char *hdmi_colorimetry_get_name(enum hdmi_colorimetry colorimetry) return "Invalid"; } -static const char * +const char * hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect) { switch (picture_aspect) { @@ -1076,6 +1076,7 @@ hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect) } return "Invalid"; } +EXPORT_SYMBOL(hdmi_picture_aspect_get_name); static const char * hdmi_active_aspect_get_name(enum hdmi_active_aspect active_aspect) diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 3962dbf82100..0a724874fd84 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -436,7 +436,7 @@ struct drm_display_mode { /** * DRM_MODE_FMT - printf string for &struct drm_display_mode */ -#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s" +#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s %s" /** * DRM_MODE_ARG - printf arguments for &struct drm_display_mode @@ -448,7 +448,9 @@ struct drm_display_mode { (m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \ (m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \ (m)->type, (m)->flags, \ - drm_get_mode_flags_name(b, sizeof(b), (m)->flags) + drm_get_mode_flags_name(b, sizeof(b), (m)->flags), \ + (m)->picture_aspect_ratio ? \ + hdmi_picture_aspect_get_name((m)->picture_aspect_ratio) : "" #define obj_to_mode(x) container_of(x, struct drm_display_mode, base) diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index 9918a6c910c5..de7cbe385dba 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -434,4 +434,7 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, void hdmi_infoframe_log(const char *level, struct device *dev, const union hdmi_infoframe *frame); +const char * +hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect); + #endif /* _DRM_HDMI_H */ -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: Pretty print mode flags
From: Ville Syrjälä Decode the mode flags when printing the modeline so that I no longer have to decode the hex number myself. To do this neatly I made the caller provide a temporary on stack buffer where we can produce the results. I choce 64 bytes as a reasonable size for this. The worst case I think is > 100 bytes but that kind of mode would be nonsense anyway so I figured correct decoding isn't as important in such cases. Cc: Russell King Cc: Neil Armstrong Cc: Rob Clark Cc: Tomi Valkeinen Cc: Thierry Reding Cc: Sam Ravnborg Cc: Benjamin Gaignard Cc: Vincent Abriou Cc: linux-amlo...@lists.infradead.org Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/armada/armada_crtc.c | 3 +- drivers/gpu/drm/drm_atomic.c | 3 +- drivers/gpu/drm/drm_modes.c | 116 +- drivers/gpu/drm/i915/i915_debugfs.c | 3 +- drivers/gpu/drm/meson/meson_dw_hdmi.c | 3 +- drivers/gpu/drm/meson/meson_venc.c| 4 +- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 3 +- .../gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c | 3 +- .../gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c | 3 +- .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 3 +- .../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c | 4 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 3 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c | 3 +- drivers/gpu/drm/msm/dsi/dsi_manager.c | 3 +- drivers/gpu/drm/msm/edp/edp_bridge.c | 3 +- drivers/gpu/drm/omapdrm/omap_connector.c | 5 +- drivers/gpu/drm/omapdrm/omap_crtc.c | 3 +- drivers/gpu/drm/panel/panel-ronbo-rb070d30.c | 3 +- drivers/gpu/drm/sti/sti_crtc.c| 3 +- include/drm/drm_modes.h | 14 ++- 20 files changed, 165 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index ba4a3fab7745..ce9335682bd2 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -262,6 +262,7 @@ static void armada_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) unsigned long flags; unsigned i; bool interlaced = !!(adj->flags & DRM_MODE_FLAG_INTERLACE); + char buf[DRM_MODE_FLAGS_BUF_LEN]; i = 0; rm = adj->crtc_hsync_start - adj->crtc_hdisplay; @@ -270,7 +271,7 @@ static void armada_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) tm = adj->crtc_vtotal - adj->crtc_vsync_end; DRM_DEBUG_KMS("[CRTC:%d:%s] mode " DRM_MODE_FMT "\n", - crtc->base.id, crtc->name, DRM_MODE_ARG(adj)); + crtc->base.id, crtc->name, DRM_MODE_ARG(adj, buf)); DRM_DEBUG_KMS("lm %d rm %d tm %d bm %d\n", lm, rm, tm, bm); /* Now compute the divider for real */ diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 419381abbdd1..81caf91fbd72 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -380,6 +380,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p, const struct drm_crtc_state *state) { struct drm_crtc *crtc = state->crtc; + char buf[DRM_MODE_FLAGS_BUF_LEN]; drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name); drm_printf(p, "\tenable=%d\n", state->enable); @@ -393,7 +394,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p, drm_printf(p, "\tplane_mask=%x\n", state->plane_mask); drm_printf(p, "\tconnector_mask=%x\n", state->connector_mask); drm_printf(p, "\tencoder_mask=%x\n", state->encoder_mask); - drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(&state->mode)); + drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(&state->mode, buf)); if (crtc->funcs->atomic_print_state) crtc->funcs->atomic_print_state(p, state); diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 57e6408288c8..3d15c600295a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -45,6 +45,118 @@ #include "drm_crtc_internal.h" +static char *snprint_cont(char *buf, int *len, + const char *str, bool last) +{ + int r; + + r = snprintf(buf, *len, "%s%s", str, last ? "" : ","); + if (r >= *len) + return buf; + + *len -= r; + buf += r; + + return buf; +} + +#define MODE_STR(x) { .name = #x, .flag = DRM_MODE_FLAG_ ## x, } + +static const struct { + const char *name; + u32 flag; +} mode_flags[] = { + MODE_STR(PHSYNC), + MODE_STR(NHSYNC), + MODE_STR(PVSYNC), + MODE_STR(NVSYNC), + MODE_STR(INTERLACE), + MODE_STR(CSYNC), + MODE_STR(PCSYNC), + MODE_STR(NCSYNC), + MODE_STR(DBLSCAN), + MODE_STR(HSKEW), + MODE_ST
[PATCH 00/19] drm/i915: Plane cdclk requirements and fp16 for gen4+
From: Ville Syrjälä The end of the series is just a reposting of the fp16 stuff for gen4+. The start of the series is new stuff to allow planes to dictate the minimum cdclk, which is sometimes needed for downscaling or fp16 (and sometimes even for other pixel formats). Thanks to that new code the fp16 tests should now pass on bxt/glk. Ville Syrjälä (19): drm: Add drm_modeset_lock_assert_held() drm/atomic-helper: Make crtc helper funcs optional drm/i915: Remove pointless planes_changed=true assignment drm/i915: Replace is_planar_yuv_format() with drm_format_info_is_yuv_semiplanar() drm/i915: Allow downscale factor of <3.0 on glk+ for all formats drm/i915: Extract intel_modeset_calc_cdclk() drm/i915: s/pipe_config/crtc_state/ in intel_crtc_atomic_check() drm/i915: Stop using drm_atomic_helper_check_planes() drm/i915: Add debugs to distingiush a cd2x update from a full cdclk pll update drm/i915: Make .modeset_calc_cdclk() mandatory drm/i915: Rework global state locking drm/i915: Move check_digital_port_conflicts() earier drm/i915: Allow planes to declare their minimum acceptable cdclk drm/i915: Eliminate skl_check_pipe_max_pixel_rate() drm/i915: Simplify skl_max_scale() drm/i915: Add support for half float framebuffers for skl+ drm/i915: Add support for half float framebuffers for gen4+ primary planes drm/i915: Add support for half float framebuffers for ivb+ sprites drm/i915: Add support for half float framebuffers on snb sprites drivers/gpu/drm/drm_atomic_helper.c | 2 +- drivers/gpu/drm/i915/display/intel_atomic.c | 34 +- drivers/gpu/drm/i915/display/intel_atomic.h | 3 + .../gpu/drm/i915/display/intel_atomic_plane.c | 45 +- .../gpu/drm/i915/display/intel_atomic_plane.h | 4 + drivers/gpu/drm/i915/display/intel_audio.c| 10 +- drivers/gpu/drm/i915/display/intel_cdclk.c| 221 +++- drivers/gpu/drm/i915/display/intel_cdclk.h| 6 +- drivers/gpu/drm/i915/display/intel_display.c | 534 ++ drivers/gpu/drm/i915/display/intel_sprite.c | 441 ++- drivers/gpu/drm/i915/display/intel_sprite.h | 8 +- drivers/gpu/drm/i915/i915_drv.h | 11 +- drivers/gpu/drm/i915/intel_drv.h | 14 +- drivers/gpu/drm/i915/intel_pm.c | 116 +--- drivers/gpu/drm/i915/intel_pm.h | 2 - include/drm/drm_modeset_lock.h| 9 + 16 files changed, 1054 insertions(+), 406 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/19] drm: Add drm_modeset_lock_assert_held()
From: Ville Syrjälä Add a small wrapper around lockdep_assert_held() to make it a bit more conventinet to use with modeset locks. Signed-off-by: Ville Syrjälä --- include/drm/drm_modeset_lock.h | 9 + 1 file changed, 9 insertions(+) diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index 7b8841065b11..4fc9a43ac45a 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -114,6 +114,15 @@ static inline bool drm_modeset_is_locked(struct drm_modeset_lock *lock) return ww_mutex_is_locked(&lock->mutex); } +/** + * drm_modeset_lock_assert_held - equivalent to lockdep_assert_held() + * @lock: lock to check + */ +static inline void drm_modeset_lock_assert_held(struct drm_modeset_lock *lock) +{ + lockdep_assert_held(&lock->mutex.base); +} + int drm_modeset_lock(struct drm_modeset_lock *lock, struct drm_modeset_acquire_ctx *ctx); int __must_check drm_modeset_lock_single_interruptible(struct drm_modeset_lock *lock); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/19] drm/atomic-helper: Make crtc helper funcs optional
From: Ville Syrjälä Allow drivers to call drm_atomic_helper_check_modeset() without having the crtc helper funcs specified. i915 doesn't need those anymore. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_atomic_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index aa16ea17ff9b..fb2ce692ae5b 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -481,7 +481,7 @@ mode_fixup(struct drm_atomic_state *state) continue; funcs = crtc->helper_private; - if (!funcs->mode_fixup) + if (!funcs || !funcs->mode_fixup) continue; ret = funcs->mode_fixup(crtc, &new_crtc_state->mode, -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/19] drm/i915: Remove pointless planes_changed=true assignment
From: Ville Syrjälä i915 doesn't use the crtc_state->plane_changed flag for anything, so setting it is pointless. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_atomic.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c index 90ca11a4ae88..954d4a930864 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic.c +++ b/drivers/gpu/drm/i915/display/intel_atomic.c @@ -378,13 +378,6 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, plane->base.id); return PTR_ERR(state); } - - /* -* the plane is added after plane checks are run, -* but since this plane is unchanged just do the -* minimum required validation. -*/ - crtc_state->base.planes_changed = true; } intel_plane = to_intel_plane(plane); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/19] drm/i915: Replace is_planar_yuv_format() with drm_format_info_is_yuv_semiplanar()
From: Ville Syrjälä There's a helper in drm_fourcc.h these days to check of we're dealing with a two plane YUV format. Make use if it. Also s/plane/color_plane/ in skl_plane_relative_data_rate() to reduce the confusion. Signed-off-by: Ville Syrjälä --- .../gpu/drm/i915/display/intel_atomic_plane.c | 5 ++-- drivers/gpu/drm/i915/display/intel_display.c | 10 +++ drivers/gpu/drm/i915/display/intel_sprite.c | 20 +++--- drivers/gpu/drm/i915/display/intel_sprite.h | 1 - drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_pm.c | 27 +-- 6 files changed, 26 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index ab411d5e093c..7ff19b524f9d 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -143,6 +143,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ struct intel_plane_state *new_plane_state) { struct intel_plane *plane = to_intel_plane(new_plane_state->base.plane); + const struct drm_framebuffer *fb = new_plane_state->base.fb; int ret; new_crtc_state->active_planes &= ~BIT(plane->id); @@ -163,11 +164,11 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ new_crtc_state->active_planes |= BIT(plane->id); if (new_plane_state->base.visible && - is_planar_yuv_format(new_plane_state->base.fb->format->format)) + drm_format_info_is_yuv_semiplanar(fb->format)) new_crtc_state->nv12_planes |= BIT(plane->id); if (new_plane_state->base.visible && - new_plane_state->base.fb->format->format == DRM_FORMAT_C8) + fb->format->format == DRM_FORMAT_C8) new_crtc_state->c8_planes |= BIT(plane->id); if (new_plane_state->base.visible || old_plane_state->base.visible) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index f09eda75711a..ee93577bdf95 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3540,7 +3540,7 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state) * Handle the AUX surface first since * the main surface setup depends on it. */ - if (is_planar_yuv_format(fb->format->format)) { + if (drm_format_info_is_yuv_semiplanar(fb->format)) { ret = skl_check_nv12_aux_surface(plane_state); if (ret) return ret; @@ -5457,7 +5457,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, return 0; } - if (format && is_planar_yuv_format(format->format) && + if (format && drm_format_info_is_yuv_semiplanar(format) && (src_h < SKL_MIN_YUV_420_SRC_H || src_w < SKL_MIN_YUV_420_SRC_W)) { DRM_DEBUG_KMS("Planar YUV: src dimensions not met\n"); return -EINVAL; @@ -5534,7 +5534,7 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, /* Pre-gen11 and SDR planes always need a scaler for planar formats. */ if (!icl_is_hdr_plane(dev_priv, intel_plane->id) && - fb && is_planar_yuv_format(fb->format->format)) + fb && drm_format_info_is_yuv_semiplanar(fb->format)) need_scaler = true; ret = skl_update_scaler(crtc_state, force_detach, @@ -14468,7 +14468,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane, int skl_max_scale(const struct intel_crtc_state *crtc_state, - u32 pixel_format) + const struct drm_format_info *format) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -14493,7 +14493,7 @@ skl_max_scale(const struct intel_crtc_state *crtc_state, *or *cdclk/crtc_clock */ - mult = is_planar_yuv_format(pixel_format) ? 2 : 3; + mult = drm_format_info_is_yuv_semiplanar(format) ? 2 : 3; tmpclk1 = (1 << 16) * mult - 1; tmpclk2 = (1 << 8) * ((max_dotclk << 8) / crtc_clock); max_scale = min(tmpclk1, tmpclk2); diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c index 34586f29be60..e89f3f7fae05 100644 --- a/drivers/gpu/drm/i915/display/intel_sprite.c +++ b/drivers/gpu/drm/i915/display/intel_sprite.c @@ -47,19 +47,6 @@ #include "intel_psr.h" #include "intel_sprite.h" -bool is_planar_yuv_format(u32 pixelformat) -{ - switch (pixelformat) { - case DRM_FORMAT_NV12: - case DRM_FORMAT_P010: - case DRM_FORMAT_P012: - case DRM_FORMAT_P016: - return true;
[PATCH 05/19] drm/i915: Allow downscale factor of <3.0 on glk+ for all formats
From: Ville Syrjälä Bspec says that glk+ max downscale factor is <3.0 for all pixel formats. Older platforms had a max of <2.0 for NV12. Update the code to deal with this. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index ee93577bdf95..2b8a6a84605c 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -14472,7 +14472,7 @@ skl_max_scale(const struct intel_crtc_state *crtc_state, { struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - int max_scale, mult; + int max_scale; int crtc_clock, max_dotclk, tmpclk1, tmpclk2; if (!crtc_state->base.enable) @@ -14493,8 +14493,11 @@ skl_max_scale(const struct intel_crtc_state *crtc_state, *or *cdclk/crtc_clock */ - mult = drm_format_info_is_yuv_semiplanar(format) ? 2 : 3; - tmpclk1 = (1 << 16) * mult - 1; + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) || + !drm_format_info_is_yuv_semiplanar(format)) + tmpclk1 = 0x3 - 1; + else + tmpclk1 = 0x2 - 1; tmpclk2 = (1 << 8) * ((max_dotclk << 8) / crtc_clock); max_scale = min(tmpclk1, tmpclk2); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/19] drm/i915: s/pipe_config/crtc_state/ in intel_crtc_atomic_check()
From: Ville Syrjälä Clean up the mess with the drm vs. intel types in intel_crtc_atomic_check() and rename varibles accordingly. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 54 ++-- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 3efe76c2ec33..5635f2079e4c 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -11733,25 +11733,24 @@ static bool c8_planes_changed(const struct intel_crtc_state *new_crtc_state) return !old_crtc_state->c8_planes != !new_crtc_state->c8_planes; } -static int intel_crtc_atomic_check(struct drm_crtc *crtc, - struct drm_crtc_state *crtc_state) +static int intel_crtc_atomic_check(struct drm_crtc *_crtc, + struct drm_crtc_state *_crtc_state) { - struct drm_i915_private *dev_priv = to_i915(crtc->dev); - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_crtc_state *pipe_config = - to_intel_crtc_state(crtc_state); + struct intel_crtc *crtc = to_intel_crtc(_crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + struct intel_crtc_state *crtc_state = + to_intel_crtc_state(_crtc_state); int ret; - bool mode_changed = needs_modeset(pipe_config); + bool mode_changed = needs_modeset(crtc_state); if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv) && - mode_changed && !crtc_state->active) - pipe_config->update_wm_post = true; + mode_changed && !crtc_state->base.active) + crtc_state->update_wm_post = true; - if (mode_changed && crtc_state->enable && + if (mode_changed && crtc_state->base.enable && dev_priv->display.crtc_compute_clock && - !WARN_ON(pipe_config->shared_dpll)) { - ret = dev_priv->display.crtc_compute_clock(intel_crtc, - pipe_config); + !WARN_ON(crtc_state->shared_dpll)) { + ret = dev_priv->display.crtc_compute_clock(crtc, crtc_state); if (ret) return ret; } @@ -11760,19 +11759,19 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, * May need to update pipe gamma enable bits * when C8 planes are getting enabled/disabled. */ - if (c8_planes_changed(pipe_config)) - crtc_state->color_mgmt_changed = true; + if (c8_planes_changed(crtc_state)) + crtc_state->base.color_mgmt_changed = true; - if (mode_changed || pipe_config->update_pipe || - crtc_state->color_mgmt_changed) { - ret = intel_color_check(pipe_config); + if (mode_changed || crtc_state->update_pipe || + crtc_state->base.color_mgmt_changed) { + ret = intel_color_check(crtc_state); if (ret) return ret; } ret = 0; if (dev_priv->display.compute_pipe_wm) { - ret = dev_priv->display.compute_pipe_wm(pipe_config); + ret = dev_priv->display.compute_pipe_wm(crtc_state); if (ret) { DRM_DEBUG_KMS("Target pipe watermarks are invalid\n"); return ret; @@ -11788,7 +11787,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, * old state and the new state. We can program these * immediately. */ - ret = dev_priv->display.compute_intermediate_wm(pipe_config); + ret = dev_priv->display.compute_intermediate_wm(crtc_state); if (ret) { DRM_DEBUG_KMS("No valid intermediate pipe watermarks are possible\n"); return ret; @@ -11796,21 +11795,20 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, } if (INTEL_GEN(dev_priv) >= 9) { - if (mode_changed || pipe_config->update_pipe) - ret = skl_update_scaler_crtc(pipe_config); + if (mode_changed || crtc_state->update_pipe) + ret = skl_update_scaler_crtc(crtc_state); if (!ret) - ret = icl_check_nv12_planes(pipe_config); + ret = icl_check_nv12_planes(crtc_state); if (!ret) - ret = skl_check_pipe_max_pixel_rate(intel_crtc, - pipe_config); + ret = skl_check_pipe_max_pixel_rate(crtc, crtc_state); if (!ret) - ret = intel_atomic_setup_scalers(dev_priv, intel_crtc, -
[PATCH 06/19] drm/i915: Extract intel_modeset_calc_cdclk()
From: Ville Syrjälä Exfiltrate the cdclk code from intel_modeset_checks() into intel_modeset_calc_cdclk(). Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 135 ++- drivers/gpu/drm/i915/display/intel_cdclk.h | 6 +- drivers/gpu/drm/i915/display/intel_display.c | 119 +--- 3 files changed, 135 insertions(+), 125 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 0b8b8ae3b7fc..c7c46b382738 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -21,6 +21,7 @@ * DEALINGS IN THE SOFTWARE. */ +#include "intel_atomic.h" #include "intel_cdclk.h" #include "intel_drv.h" #include "intel_sideband.h" @@ -2081,9 +2082,9 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a, * Returns: * True if the CDCLK states require just a cd2x divider update, false if not. */ -bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv, - const struct intel_cdclk_state *a, - const struct intel_cdclk_state *b) +static bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv, + const struct intel_cdclk_state *a, + const struct intel_cdclk_state *b) { /* Older hw doesn't have the capability */ if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv)) @@ -2102,8 +2103,8 @@ bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv, * Returns: * True if the CDCLK states don't match, false if they do. */ -bool intel_cdclk_changed(const struct intel_cdclk_state *a, -const struct intel_cdclk_state *b) +static bool intel_cdclk_changed(const struct intel_cdclk_state *a, + const struct intel_cdclk_state *b) { return intel_cdclk_needs_modeset(a, b) || a->voltage_level != b->voltage_level; @@ -2556,6 +2557,62 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state) return 0; } +static int intel_lock_all_pipes(struct intel_atomic_state *state) +{ + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + struct intel_crtc *crtc; + + /* Add all pipes to the state */ + for_each_intel_crtc(&dev_priv->drm, crtc) { + struct intel_crtc_state *crtc_state; + + crtc_state = intel_atomic_get_crtc_state(&state->base, crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + } + + return 0; +} + +static int intel_modeset_all_pipes(struct intel_atomic_state *state) +{ + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + struct intel_crtc *crtc; + + /* +* Add all pipes to the state, and force +* a modeset on all the active ones. +*/ + for_each_intel_crtc(&dev_priv->drm, crtc) { + struct intel_crtc_state *crtc_state; + int ret; + + crtc_state = intel_atomic_get_crtc_state(&state->base, crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + + if (!crtc_state->base.active || + drm_atomic_crtc_needs_modeset(&crtc_state->base)) + continue; + + crtc_state->base.mode_changed = true; + + ret = drm_atomic_add_affected_connectors(&state->base, +&crtc->base); + if (ret) + return ret; + + ret = drm_atomic_add_affected_planes(&state->base, +&crtc->base); + if (ret) + return ret; + + crtc_state->update_planes |= crtc_state->active_planes; + } + + return 0; +} + static int icl_modeset_calc_cdclk(struct intel_atomic_state *state) { struct drm_i915_private *dev_priv = to_i915(state->base.dev); @@ -2590,6 +2647,74 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state) return 0; } +int intel_modeset_calc_cdclk(struct intel_atomic_state *state) +{ + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + enum pipe pipe; + int ret; + + if (!dev_priv->display.modeset_calc_cdclk) + return 0; + + ret = dev_priv->display.modeset_calc_cdclk(state); + if (ret) + return ret; + + /* +* Writes to dev_priv->cdclk.logical must protected by +* holding all the crtc locks, even if we don't end up +* touching the hardware +*/ + if (intel_cdclk_changed(&dev_priv->cdclk.logical, + &state->cdclk.logical)) { + ret = intel_lock_all_pipes
[PATCH 11/19] drm/i915: Rework global state locking
From: Ville Syrjälä So far we've sort of protected the global state under dev_priv with the connection_mutex. I wan to change that so that we can change the cdclk even for pure plane updates. To that end let's formalize the protection of the global state to follow what I started with the cdclk code already (though not entirely properly) such that any crtc mutex will suffice as a read lock, and all crtcs mutexes act as the write lock. We'll also pimp intel_atomic_state_clear() to clear the entire global state, so that we don't accidentally leak stale information between the locking retries. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_atomic.c | 27 + drivers/gpu/drm/i915/display/intel_atomic.h | 3 + drivers/gpu/drm/i915/display/intel_audio.c | 10 +- drivers/gpu/drm/i915/display/intel_cdclk.c | 116 ++- drivers/gpu/drm/i915/display/intel_display.c | 29 - drivers/gpu/drm/i915/i915_drv.h | 11 +- drivers/gpu/drm/i915/intel_drv.h | 8 ++ 7 files changed, 139 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c index 954d4a930864..de4cd482dbe5 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic.c +++ b/drivers/gpu/drm/i915/display/intel_atomic.c @@ -418,6 +418,13 @@ void intel_atomic_state_clear(struct drm_atomic_state *s) struct intel_atomic_state *state = to_intel_atomic_state(s); drm_atomic_state_default_clear(&state->base); state->dpll_set = state->modeset = false; + state->global_state_changed = false; + state->active_crtcs = 0; + memset(&state->min_cdclk, 0, sizeof(state->min_cdclk)); + memset(&state->min_voltage_level, 0, sizeof(state->min_voltage_level)); + memset(&state->cdclk.logical, 0, sizeof(state->cdclk.logical)); + memset(&state->cdclk.actual, 0, sizeof(state->cdclk.actual)); + state->cdclk.pipe = INVALID_PIPE; } struct intel_crtc_state * @@ -431,3 +438,23 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state, return to_intel_crtc_state(crtc_state); } + +int intel_atomic_lock_global_state(struct intel_atomic_state *state) +{ + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + struct intel_crtc *crtc; + + state->global_state_changed = true; + + /* Lock all crtc mutexes */ + for_each_intel_crtc(&dev_priv->drm, crtc) { + int ret; + + ret = drm_modeset_lock(&crtc->base.mutex, + state->base.acquire_ctx); + if (ret) + return ret; + } + + return 0; +} diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h b/drivers/gpu/drm/i915/display/intel_atomic.h index 58065d3161a3..0d6cd22b7e5f 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic.h +++ b/drivers/gpu/drm/i915/display/intel_atomic.h @@ -16,6 +16,7 @@ struct drm_crtc_state; struct drm_device; struct drm_i915_private; struct drm_property; +struct intel_atomic_state; struct intel_crtc; struct intel_crtc_state; @@ -46,4 +47,6 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state); +int intel_atomic_lock_global_state(struct intel_atomic_state *state); + #endif /* __INTEL_ATOMIC_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index c8fd35a7ca42..22ccb824c716 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -28,6 +28,7 @@ #include #include "i915_drv.h" +#include "intel_atomic.h" #include "intel_audio.h" #include "intel_drv.h" #include "intel_lpe_audio.h" @@ -816,13 +817,8 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv, to_intel_atomic_state(state)->cdclk.force_min_cdclk = enable ? 2 * 96000 : 0; - /* -* Protects dev_priv->cdclk.force_min_cdclk -* Need to lock this here in case we have no active pipes -* and thus wouldn't lock it during the commit otherwise. -*/ - ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex, - &ctx); + /* Protects dev_priv->cdclk.force_min_cdclk */ + ret = intel_atomic_lock_global_state(to_intel_atomic_state(state)); if (!ret) ret = drm_atomic_commit(state); diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 4649485fee33..40583d8d259b 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -442,7 +442,7 @@ static int vlv_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk) return 20; } -static u8 vlv_calc_voltage_level(s
[PATCH 10/19] drm/i915: Make .modeset_calc_cdclk() mandatory
From: Ville Syrjälä While not all platforms allow us to change the cdclk frequency we should still verify that the fixed cdclk frequency isn't too low. To that end let's cook up a .modeset_calc_cdclk() implementation that only does the min_cdclk vs. actual cdclk frequency check for such platforms. Also we mustn't forget about double wide pipe on gen2/3 when doing this. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 31 drivers/gpu/drm/i915/display/intel_display.c | 8 ++--- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index f2910c0c3e3e..4649485fee33 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2209,9 +2209,11 @@ intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv, intel_set_cdclk(dev_priv, new_state, pipe); } -static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv, -int pixel_rate) +static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state *crtc_state) { + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); + int pixel_rate = crtc_state->pixel_rate; + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) return DIV_ROUND_UP(pixel_rate, 2); else if (IS_GEN(dev_priv, 9) || @@ -2219,6 +2221,8 @@ static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv, return pixel_rate; else if (IS_CHERRYVIEW(dev_priv)) return DIV_ROUND_UP(pixel_rate * 100, 95); + else if (crtc_state->double_wide) + return DIV_ROUND_UP(pixel_rate * 100, 90 * 2); else return DIV_ROUND_UP(pixel_rate * 100, 90); } @@ -2232,7 +2236,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) if (!crtc_state->base.enable) return 0; - min_cdclk = intel_pixel_rate_to_cdclk(dev_priv, crtc_state->pixel_rate); + min_cdclk = intel_pixel_rate_to_cdclk(crtc_state); /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ if (IS_BROADWELL(dev_priv) && hsw_crtc_state_ips_capable(crtc_state)) @@ -2647,15 +2651,28 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state) return 0; } +static int fixed_modeset_calc_cdclk(struct intel_atomic_state *state) +{ + int min_cdclk; + + /* +* We can't change the cdclk frequency, but we still want to +* check that the required minimum frequency doesn't exceed +* the actual cdclk frequency. +*/ + min_cdclk = intel_compute_min_cdclk(state); + if (min_cdclk < 0) + return min_cdclk; + + return 0; +} + int intel_modeset_calc_cdclk(struct intel_atomic_state *state) { struct drm_i915_private *dev_priv = to_i915(state->base.dev); enum pipe pipe; int ret; - if (!dev_priv->display.modeset_calc_cdclk) - return 0; - ret = dev_priv->display.modeset_calc_cdclk(state); if (ret) return ret; @@ -2957,6 +2974,8 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) } else if (IS_VALLEYVIEW(dev_priv)) { dev_priv->display.set_cdclk = vlv_set_cdclk; dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk; + } else { + dev_priv->display.modeset_calc_cdclk = fixed_modeset_calc_cdclk; } if (INTEL_GEN(dev_priv) >= 11) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 5a42cbfa72c3..2d3cfdc80fd3 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -16701,11 +16701,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) intel_crtc_compute_pixel_rate(crtc_state); - if (dev_priv->display.modeset_calc_cdclk) { - min_cdclk = intel_crtc_compute_min_cdclk(crtc_state); - if (WARN_ON(min_cdclk < 0)) - min_cdclk = 0; - } + min_cdclk = intel_crtc_compute_min_cdclk(crtc_state); + if (WARN_ON(min_cdclk < 0)) + min_cdclk = 0; drm_calc_timestamping_constants(&crtc->base, &crtc_state->base.adjusted_mode); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 08/19] drm/i915: Stop using drm_atomic_helper_check_planes()
From: Ville Syrjälä We need to insert stuff between the plane and crtc .atomic_check() drm_atomic_helper_check_planes() doesn't allow us to do that so stop using it and hand roll the loops instead. Signed-off-by: Ville Syrjälä --- .../gpu/drm/i915/display/intel_atomic_plane.c | 10 +--- .../gpu/drm/i915/display/intel_atomic_plane.h | 2 + drivers/gpu/drm/i915/display/intel_display.c | 57 +++ 3 files changed, 50 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 7ff19b524f9d..d7493551b28c 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -194,14 +194,11 @@ get_crtc_from_states(const struct intel_plane_state *old_plane_state, return NULL; } -static int intel_plane_atomic_check(struct drm_plane *_plane, - struct drm_plane_state *_new_plane_state) +int intel_plane_atomic_check(struct intel_atomic_state *state, +struct intel_plane *plane) { - struct intel_plane *plane = to_intel_plane(_plane); - struct intel_atomic_state *state = - to_intel_atomic_state(_new_plane_state->state); struct intel_plane_state *new_plane_state = - to_intel_plane_state(_new_plane_state); + intel_atomic_get_new_plane_state(state, plane); const struct intel_plane_state *old_plane_state = intel_atomic_get_old_plane_state(state, plane); struct intel_crtc *crtc = @@ -368,5 +365,4 @@ void i9xx_update_planes_on_crtc(struct intel_atomic_state *state, const struct drm_plane_helper_funcs intel_plane_helper_funcs = { .prepare_fb = intel_prepare_plane_fb, .cleanup_fb = intel_cleanup_plane_fb, - .atomic_check = intel_plane_atomic_check, }; diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index cb7ef4f9eafd..dc85af02e9b7 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -41,6 +41,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ struct intel_crtc_state *crtc_state, const struct intel_plane_state *old_plane_state, struct intel_plane_state *intel_state); +int intel_plane_atomic_check(struct intel_atomic_state *state, +struct intel_plane *plane); int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_state, struct intel_crtc_state *crtc_state, const struct intel_plane_state *old_plane_state, diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 5635f2079e4c..5a42cbfa72c3 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -11733,15 +11733,14 @@ static bool c8_planes_changed(const struct intel_crtc_state *new_crtc_state) return !old_crtc_state->c8_planes != !new_crtc_state->c8_planes; } -static int intel_crtc_atomic_check(struct drm_crtc *_crtc, - struct drm_crtc_state *_crtc_state) +static int intel_crtc_atomic_check(struct intel_atomic_state *state, + struct intel_crtc *crtc) { - struct intel_crtc *crtc = to_intel_crtc(_crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); struct intel_crtc_state *crtc_state = - to_intel_crtc_state(_crtc_state); - int ret; + intel_atomic_get_new_crtc_state(state, crtc); bool mode_changed = needs_modeset(crtc_state); + int ret; if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv) && mode_changed && !crtc_state->base.active) @@ -11813,10 +11812,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *_crtc, return ret; } -static const struct drm_crtc_helper_funcs intel_helper_funcs = { - .atomic_check = intel_crtc_atomic_check, -}; - static void intel_modeset_update_connector_atomic_state(struct drm_device *dev) { struct intel_connector *connector; @@ -13457,6 +13452,42 @@ static void intel_crtc_check_fastset(const struct intel_crtc_state *old_crtc_sta new_crtc_state->has_drrs = old_crtc_state->has_drrs; } +static int intel_atomic_check_planes(struct intel_atomic_state *state) +{ + struct intel_plane_state *plane_state; + struct intel_plane *plane; + int i, ret; + + for_each_new_intel_plane_in_state(state, plane, plane_state, i) { + ret = intel_plane_atomic_check(state, plane); + if (ret) { + DRM_DEBUG_ATOMIC("[PLANE
[PATCH 13/19] drm/i915: Allow planes to declare their minimum acceptable cdclk
From: Ville Syrjälä Various pixel formats and plane scaling impose additional constraints on the cdclk frequency. Provide a new plane->min_cdclk() hook that will be used to compute the minimum acceptable cdclk frequency for each plane. Annoyingly on some platforms the numer of active planes affects this calculation so we must also toss in more planes into the state when the number of active planes changes. The sequence of state computation must also be changed: 1. check_plane() (updates plane's visibility etc.) 2. figure out if more planes now require update min_cdclk computaion 3. calculate the new min cdclk for each plane in the state 4. if the minimum of any plane now exceeds the current logical cdclk we recompute the cdclk 4. during cdclk computation take the planes' min_cdclk into accoutn 5. follow the normal cdclk programming to change the cdclk frequency. This may now require a modeset (except on bxt/glk in some cases), which either succeeds or fails depending on whether userspace has given us permission to perform a modeset or not. Signed-off-by: Ville Syrjälä --- .../gpu/drm/i915/display/intel_atomic_plane.c | 30 ++ .../gpu/drm/i915/display/intel_atomic_plane.h | 2 + drivers/gpu/drm/i915/display/intel_cdclk.c| 16 + drivers/gpu/drm/i915/display/intel_display.c | 187 -- drivers/gpu/drm/i915/display/intel_sprite.c | 342 ++ drivers/gpu/drm/i915/display/intel_sprite.h | 7 + drivers/gpu/drm/i915/intel_drv.h | 4 + 7 files changed, 566 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index d7493551b28c..91e841d38086 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -137,6 +137,35 @@ unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state, return cpp * crtc_state->pixel_rate; } +bool intel_plane_calc_min_cdclk(struct intel_atomic_state *state, + struct intel_plane *plane) +{ + const struct intel_plane_state *plane_state = + intel_atomic_get_new_plane_state(state, plane); + struct intel_crtc *crtc = to_intel_crtc(plane_state->base.crtc); + struct intel_crtc_state *crtc_state; + + if (!plane_state->base.visible || !plane->min_cdclk) + return false; + + crtc_state = intel_atomic_get_new_crtc_state(state, crtc); + + crtc_state->min_cdclk[plane->id] = + plane->min_cdclk(crtc_state, plane_state); + + DRM_DEBUG_KMS("[PLANE:%d:%s] min_cdclk %d kHz (logical cdclk %d kHz)\n", + plane->base.base.id, plane->base.name, + crtc_state->min_cdclk[plane->id], + state->cdclk.logical.cdclk); + + /* should have been populated in intel_atomic_check_planes() */ + WARN_ON(state->cdclk.logical.cdclk == 0); + + /* Does the cdclk need to be bumbed up? */ + return crtc_state->min_cdclk[plane->id] > + state->cdclk.logical.cdclk; +} + int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_state, struct intel_crtc_state *new_crtc_state, const struct intel_plane_state *old_plane_state, @@ -150,6 +179,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ new_crtc_state->nv12_planes &= ~BIT(plane->id); new_crtc_state->c8_planes &= ~BIT(plane->id); new_crtc_state->data_rate[plane->id] = 0; + new_crtc_state->min_cdclk[plane->id] = 0; new_plane_state->base.visible = false; if (!new_plane_state->base.crtc && !old_plane_state->base.crtc) diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index dc85af02e9b7..e61e9a82aadf 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -47,5 +47,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat struct intel_crtc_state *crtc_state, const struct intel_plane_state *old_plane_state, struct intel_plane_state *plane_state); +bool intel_plane_calc_min_cdclk(struct intel_atomic_state *state, + struct intel_plane *plane); #endif /* __INTEL_ATOMIC_PLANE_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 40583d8d259b..1871c56e48ea 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2227,6 +2227,19 @@ static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state *crtc_state) return DIV_ROUND_
[PATCH 14/19] drm/i915: Eliminate skl_check_pipe_max_pixel_rate()
From: Ville Syrjälä The normal cdclk handling now takes care of making sure the plane's pixel rate doesn't exceed the spec appointed percentage of the cdclk frequency. Thus we can nuke skl_check_pipe_max_pixel_rate(). Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 2 - drivers/gpu/drm/i915/intel_pm.c | 89 drivers/gpu/drm/i915/intel_pm.h | 2 - 3 files changed, 93 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 68738d177fd9..1e67fbe50476 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -11845,8 +11845,6 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state, if (INTEL_GEN(dev_priv) >= 9) { if (mode_changed || crtc_state->update_pipe) ret = skl_update_scaler_crtc(crtc_state); - if (!ret) - ret = skl_check_pipe_max_pixel_rate(crtc, crtc_state); if (!ret) ret = intel_atomic_setup_scalers(dev_priv, crtc, crtc_state); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f25a605aacf9..42775db1c7e2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4101,95 +4101,6 @@ skl_plane_downscale_amount(const struct intel_crtc_state *crtc_state, return mul_fixed16(downscale_w, downscale_h); } -static uint_fixed_16_16_t -skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) -{ - uint_fixed_16_16_t pipe_downscale = u32_to_fixed16(1); - - if (!crtc_state->base.enable) - return pipe_downscale; - - if (crtc_state->pch_pfit.enabled) { - u32 src_w, src_h, dst_w, dst_h; - u32 pfit_size = crtc_state->pch_pfit.size; - uint_fixed_16_16_t fp_w_ratio, fp_h_ratio; - uint_fixed_16_16_t downscale_h, downscale_w; - - src_w = crtc_state->pipe_src_w; - src_h = crtc_state->pipe_src_h; - dst_w = pfit_size >> 16; - dst_h = pfit_size & 0x; - - if (!dst_w || !dst_h) - return pipe_downscale; - - fp_w_ratio = div_fixed16(src_w, dst_w); - fp_h_ratio = div_fixed16(src_h, dst_h); - downscale_w = max_fixed16(fp_w_ratio, u32_to_fixed16(1)); - downscale_h = max_fixed16(fp_h_ratio, u32_to_fixed16(1)); - - pipe_downscale = mul_fixed16(downscale_w, downscale_h); - } - - return pipe_downscale; -} - -int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, - struct intel_crtc_state *crtc_state) -{ - struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); - struct drm_atomic_state *state = crtc_state->base.state; - struct drm_plane *plane; - const struct drm_plane_state *drm_plane_state; - int crtc_clock, dotclk; - u32 pipe_max_pixel_rate; - uint_fixed_16_16_t pipe_downscale; - uint_fixed_16_16_t max_downscale = u32_to_fixed16(1); - - if (!crtc_state->base.enable) - return 0; - - drm_atomic_crtc_state_for_each_plane_state(plane, drm_plane_state, &crtc_state->base) { - uint_fixed_16_16_t plane_downscale; - uint_fixed_16_16_t fp_9_div_8 = div_fixed16(9, 8); - int bpp; - const struct intel_plane_state *plane_state = - to_intel_plane_state(drm_plane_state); - - if (!intel_wm_plane_visible(crtc_state, plane_state)) - continue; - - if (WARN_ON(!plane_state->base.fb)) - return -EINVAL; - - plane_downscale = skl_plane_downscale_amount(crtc_state, plane_state); - bpp = plane_state->base.fb->format->cpp[0] * 8; - if (bpp == 64) - plane_downscale = mul_fixed16(plane_downscale, - fp_9_div_8); - - max_downscale = max_fixed16(plane_downscale, max_downscale); - } - pipe_downscale = skl_pipe_downscale_amount(crtc_state); - - pipe_downscale = mul_fixed16(pipe_downscale, max_downscale); - - crtc_clock = crtc_state->base.adjusted_mode.crtc_clock; - dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; - - if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10) - dotclk *= 2; - - pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale); - - if (pipe_max_pixel_rate < crtc_clock) { - DRM_DEBUG_KMS("Max supported pixel clock with scaling exceeded\n"); - return -EINVAL; - } - - return 0; -}
[PATCH 12/19] drm/i915: Move check_digital_port_conflicts() earier
From: Ville Syrjälä check_digital_port_conflicts() is done needlessly late. Move it earlier. This will be needed as later on we want to set any_ms=true a bit later for non-modesets too and we can't call this guy without the connection_mutex held. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 5b6300b82c50..eaed4aee4ee4 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -13380,11 +13380,6 @@ static int intel_modeset_checks(struct intel_atomic_state *state) struct intel_crtc *crtc; int ret, i; - if (!check_digital_port_conflicts(state)) { - DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n"); - return -EINVAL; - } - /* keep the current setting */ if (!state->cdclk.force_min_cdclk_changed) state->cdclk.force_min_cdclk = dev_priv->cdclk.force_min_cdclk; @@ -13546,6 +13541,12 @@ static int intel_atomic_check(struct drm_device *dev, any_ms = true; } + if (any_ms && !check_digital_port_conflicts(state)) { + DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n"); + ret = EINVAL; + goto fail; + } + ret = drm_dp_mst_atomic_check(&state->base); if (ret) goto fail; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/19] drm/i915: Add debugs to distingiush a cd2x update from a full cdclk pll update
From: Ville Syrjälä To make the logs a bit less confusing let's toss in some debug prints to indicate whether the cdclk reprogramming is going to happen with a single pipe active or whether we need to turn all pipes off for the duration. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_cdclk.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index c7c46b382738..f2910c0c3e3e 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -2696,6 +2696,9 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) return ret; state->cdclk.pipe = pipe; + + DRM_DEBUG_KMS("Can change cdclk with pipe %c active\n", + pipe_name(pipe)); } else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual, &state->cdclk.actual)) { ret = intel_modeset_all_pipes(state); @@ -2703,6 +2706,8 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state) return ret; state->cdclk.pipe = INVALID_PIPE; + + DRM_DEBUG_KMS("Modeset required for cdclk change\n"); } DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n", -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 15/19] drm/i915: Simplify skl_max_scale()
From: Ville Syrjälä Now that the planes declare their minimum cdclk requirements properly we don't need to check the cdclk in skl_max_scale() anymore. Just check against the maximum downscale ratio, and move the code next to it's only caller. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 38 drivers/gpu/drm/i915/display/intel_sprite.c | 12 ++- drivers/gpu/drm/i915/intel_drv.h | 2 -- 3 files changed, 11 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 1e67fbe50476..489620ef476b 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -14525,44 +14525,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane, mutex_unlock(&dev_priv->drm.struct_mutex); } -int -skl_max_scale(const struct intel_crtc_state *crtc_state, - const struct drm_format_info *format) -{ - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - int max_scale; - int crtc_clock, max_dotclk, tmpclk1, tmpclk2; - - if (!crtc_state->base.enable) - return DRM_PLANE_HELPER_NO_SCALING; - - crtc_clock = crtc_state->base.adjusted_mode.crtc_clock; - max_dotclk = to_intel_atomic_state(crtc_state->base.state)->cdclk.logical.cdclk; - - if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10) - max_dotclk *= 2; - - if (WARN_ON_ONCE(!crtc_clock || max_dotclk < crtc_clock)) - return DRM_PLANE_HELPER_NO_SCALING; - - /* -* skl max scale is lower of: -*close to 3 but not 3, -1 is for that purpose -*or -*cdclk/crtc_clock -*/ - if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) || - !drm_format_info_is_yuv_semiplanar(format)) - tmpclk1 = 0x3 - 1; - else - tmpclk1 = 0x2 - 1; - tmpclk2 = (1 << 8) * ((max_dotclk << 8) / crtc_clock); - max_scale = min(tmpclk1, tmpclk2); - - return max_scale; -} - static void intel_begin_crtc_commit(struct intel_atomic_state *state, struct intel_crtc *crtc) { diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c index a07887279e1a..0ffbec8291ee 100644 --- a/drivers/gpu/drm/i915/display/intel_sprite.c +++ b/drivers/gpu/drm/i915/display/intel_sprite.c @@ -2089,6 +2089,16 @@ static int skl_plane_check_nv12_rotation(const struct intel_plane_state *plane_s return 0; } +static int skl_plane_max_scale(struct drm_i915_private *dev_priv, + const struct drm_framebuffer *fb) +{ + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) || + !drm_format_info_is_yuv_semiplanar(fb->format)) + return 0x3 - 1; + else + return 0x2 - 1; +} + static int skl_plane_check(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state) { @@ -2106,7 +2116,7 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state, /* use scaler when colorkey is not required */ if (!plane_state->ckey.flags && intel_fb_scalable(fb)) { min_scale = 1; - max_scale = skl_max_scale(crtc_state, fb->format); + max_scale = skl_plane_max_scale(dev_priv, fb); } ret = drm_atomic_helper_check_plane_state(&plane_state->base, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 999ad3166cd1..02eeaec86997 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1620,8 +1620,6 @@ void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center); int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state); -int skl_max_scale(const struct intel_crtc_state *crtc_state, - const struct drm_format_info *format); static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state) { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 16/19] drm/i915: Add support for half float framebuffers for skl+
From: Ville Syrjälä skl+ supports fp16 pixel formats on all universal planes. Add the necessary bits to expose that capability. The main different to icl is that we can't scale fp16, so need to add the relevant checks. v2: Rebase on top of icl fp16 Split skl+ bits into a separate patch Reviewed-by: Maarten Lankhorst Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 11 +++ drivers/gpu/drm/i915/display/intel_sprite.c | 11 +++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 489620ef476b..e5ee6bc8720c 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5614,10 +5614,6 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, case DRM_FORMAT_ARGB: case DRM_FORMAT_XRGB2101010: case DRM_FORMAT_XBGR2101010: - case DRM_FORMAT_XBGR16161616F: - case DRM_FORMAT_ABGR16161616F: - case DRM_FORMAT_XRGB16161616F: - case DRM_FORMAT_ARGB16161616F: case DRM_FORMAT_YUYV: case DRM_FORMAT_YVYU: case DRM_FORMAT_UYVY: @@ -5633,6 +5629,13 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state, case DRM_FORMAT_XVYU12_16161616: case DRM_FORMAT_XVYU16161616: break; + case DRM_FORMAT_XBGR16161616F: + case DRM_FORMAT_ABGR16161616F: + case DRM_FORMAT_XRGB16161616F: + case DRM_FORMAT_ARGB16161616F: + if (INTEL_GEN(dev_priv) >= 11) + break; + /* fall through */ default: DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n", intel_plane->base.base.id, intel_plane->base.name, diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c index 0ffbec8291ee..6e371e68c018 100644 --- a/drivers/gpu/drm/i915/display/intel_sprite.c +++ b/drivers/gpu/drm/i915/display/intel_sprite.c @@ -1801,6 +1801,11 @@ static bool intel_fb_scalable(const struct drm_framebuffer *fb) switch (fb->format->format) { case DRM_FORMAT_C8: return false; + case DRM_FORMAT_XRGB16161616F: + case DRM_FORMAT_ARGB16161616F: + case DRM_FORMAT_XBGR16161616F: + case DRM_FORMAT_ABGR16161616F: + return INTEL_GEN(to_i915(fb->dev)) >= 11; default: return true; } @@ -2322,6 +2327,8 @@ static const u32 skl_plane_formats[] = { DRM_FORMAT_ABGR, DRM_FORMAT_XRGB2101010, DRM_FORMAT_XBGR2101010, + DRM_FORMAT_XRGB16161616F, + DRM_FORMAT_XBGR16161616F, DRM_FORMAT_YUYV, DRM_FORMAT_YVYU, DRM_FORMAT_UYVY, @@ -2337,6 +2344,8 @@ static const u32 skl_planar_formats[] = { DRM_FORMAT_ABGR, DRM_FORMAT_XRGB2101010, DRM_FORMAT_XBGR2101010, + DRM_FORMAT_XRGB16161616F, + DRM_FORMAT_XBGR16161616F, DRM_FORMAT_YUYV, DRM_FORMAT_YVYU, DRM_FORMAT_UYVY, @@ -2353,6 +2362,8 @@ static const u32 glk_planar_formats[] = { DRM_FORMAT_ABGR, DRM_FORMAT_XRGB2101010, DRM_FORMAT_XBGR2101010, + DRM_FORMAT_XRGB16161616F, + DRM_FORMAT_XBGR16161616F, DRM_FORMAT_YUYV, DRM_FORMAT_YVYU, DRM_FORMAT_UYVY, -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 17/19] drm/i915: Add support for half float framebuffers for gen4+ primary planes
From: Ville Syrjälä gen4+ supports fp16 pixel formats on the primary planes. Add the relevant code. On ivb fp16 scanout is slightly busted. The output from the plane will have 1/4 the expected value. For the primary plane we would have to use the pipe gamma or pipe csc to correct that which would affect all the other planes as well, hence we simply choose not to expose fp16 on the ivb primary plane. On hsw the primary plane got fixed. On gmch platforms I observed that the plane width must be below 2k pixels with fp16 or else we get a corrupted image. This limitation does not seem to be documented in bspec. I verified the exact limit using the chv pipe B primary plane since it has windowing capability. The stride limits are unaffected by fp16. v2: Rebase on top of icl fp16 Split thea gen4+ primary plane bits into a separate patch Deal with HAS_GMCH() Reviewed-by: Maarten Lankhorst Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_display.c | 49 ++-- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index e5ee6bc8720c..fc4af03d1113 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -88,7 +88,17 @@ static const u32 i8xx_primary_formats[] = { DRM_FORMAT_XRGB, }; -/* Primary plane formats for gen >= 4 */ +/* Primary plane formats for ivb (no fp16 due to hw issue) */ +static const u32 ivb_primary_formats[] = { + DRM_FORMAT_C8, + DRM_FORMAT_RGB565, + DRM_FORMAT_XRGB, + DRM_FORMAT_XBGR, + DRM_FORMAT_XRGB2101010, + DRM_FORMAT_XBGR2101010, +}; + +/* Primary plane formats for gen >= 4, except ivb */ static const u32 i965_primary_formats[] = { DRM_FORMAT_C8, DRM_FORMAT_RGB565, @@ -96,6 +106,7 @@ static const u32 i965_primary_formats[] = { DRM_FORMAT_XBGR, DRM_FORMAT_XRGB2101010, DRM_FORMAT_XBGR2101010, + DRM_FORMAT_XBGR16161616F, }; static const u64 i9xx_format_modifiers[] = { @@ -2982,6 +2993,8 @@ static int i9xx_format_to_fourcc(int format) return DRM_FORMAT_XRGB2101010; case DISPPLANE_RGBX101010: return DRM_FORMAT_XBGR2101010; + case DISPPLANE_RGBX161616: + return DRM_FORMAT_XBGR16161616F; } } @@ -3691,6 +3704,9 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state, case DRM_FORMAT_XBGR2101010: dspcntr |= DISPPLANE_RGBX101010; break; + case DRM_FORMAT_XBGR16161616F: + dspcntr |= DISPPLANE_RGBX161616; + break; default: MISSING_CASE(fb->format->format); return 0; @@ -3713,7 +3729,8 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(plane_state->base.plane->dev); - int src_x, src_y; + const struct drm_framebuffer *fb = plane_state->base.fb; + int src_x, src_y, src_w; u32 offset; int ret; @@ -3724,9 +3741,14 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state) if (!plane_state->base.visible) return 0; + src_w = drm_rect_width(&plane_state->base.src) >> 16; src_x = plane_state->base.src.x1 >> 16; src_y = plane_state->base.src.y1 >> 16; + /* Undocumented hardware limit on i965/g4x/vlv/chv */ + if (HAS_GMCH(dev_priv) && fb->format->cpp[0] == 8 && src_w > 2048) + return -EINVAL; + intel_add_fb_offsets(&src_x, &src_y, plane_state, 0); if (INTEL_GEN(dev_priv) >= 4) @@ -14648,6 +14670,7 @@ static bool i965_plane_format_mod_supported(struct drm_plane *_plane, case DRM_FORMAT_XBGR: case DRM_FORMAT_XRGB2101010: case DRM_FORMAT_XBGR2101010: + case DRM_FORMAT_XBGR16161616F: return modifier == DRM_FORMAT_MOD_LINEAR || modifier == I915_FORMAT_MOD_X_TILED; default: @@ -14877,8 +14900,26 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) } if (INTEL_GEN(dev_priv) >= 4) { - formats = i965_primary_formats; - num_formats = ARRAY_SIZE(i965_primary_formats); + /* +* WaFP16GammaEnabling:ivb +* "Workaround : When using the 64-bit format, the plane +* output on each color channel has one quarter amplitude. +* It can be brought up to full amplitude by using pipe +* gamma correction or pipe color space conversion to +* multiply the plane output by four." +* +* There is no dedicated plane gamma for the primary plane, +* and using the pipe gamma/csc could conflict with
[PATCH 18/19] drm/i915: Add support for half float framebuffers for ivb+ sprites
From: Ville Syrjälä ivb+ supports fp16 pixel formats on the sprite planes planes. Expose that capability. On ivb/hsw fp16 scanout is slightly busted. The output from the plane will have 1/4 the expected value. For the sprite plane we can fix that up with the plane gamma unit. This was fixed on bdw. v2: Rebase on top of icl fp16 Split the ivb+ sprite birs into a separate patch v3: Move ivb_need_sprite_gamma() check one level up so that we don't waste time programming garbage into he gamma registers Reviewed-by: Maarten Lankhorst Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_sprite.c | 48 ++--- 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c index 6e371e68c018..bd8ccc2de1da 100644 --- a/drivers/gpu/drm/i915/display/intel_sprite.c +++ b/drivers/gpu/drm/i915/display/intel_sprite.c @@ -1285,6 +1285,16 @@ static u32 ivb_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state) return sprctl; } +static bool ivb_need_sprite_gamma(const struct intel_plane_state *plane_state) +{ + struct drm_i915_private *dev_priv = + to_i915(plane_state->base.plane->dev); + const struct drm_framebuffer *fb = plane_state->base.fb; + + return fb->format->cpp[0] == 8 && + (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv)); +} + static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { @@ -1307,6 +1317,12 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state, case DRM_FORMAT_XRGB: sprctl |= SPRITE_FORMAT_RGBX888; break; + case DRM_FORMAT_XBGR16161616F: + sprctl |= SPRITE_FORMAT_RGBX161616 | SPRITE_RGB_ORDER_RGBX; + break; + case DRM_FORMAT_XRGB16161616F: + sprctl |= SPRITE_FORMAT_RGBX161616; + break; case DRM_FORMAT_YUYV: sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV; break; @@ -1324,7 +1340,8 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state, return 0; } - sprctl |= SPRITE_INT_GAMMA_DISABLE; + if (!ivb_need_sprite_gamma(plane_state)) + sprctl |= SPRITE_INT_GAMMA_DISABLE; if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709) sprctl |= SPRITE_YUV_TO_RGB_CSC_FORMAT_BT709; @@ -1346,12 +1363,26 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state, return sprctl; } -static void ivb_sprite_linear_gamma(u16 gamma[18]) +static void ivb_sprite_linear_gamma(const struct intel_plane_state *plane_state, + u16 gamma[18]) { - int i; + int scale, i; - for (i = 0; i < 17; i++) - gamma[i] = (i << 10) / 16; + /* +* WaFP16GammaEnabling:ivb,hsw +* "Workaround : When using the 64-bit format, the sprite output +* on each color channel has one quarter amplitude. It can be +* brought up to full amplitude by using sprite internal gamma +* correction, pipe gamma correction, or pipe color space +* conversion to multiply the sprite output by four." +*/ + scale = 4; + + for (i = 0; i < 16; i++) + gamma[i] = min((scale * i << 10) / 16, (1 << 10) - 1); + + gamma[i] = min((scale * i << 10) / 16, 1 << 10); + i++; gamma[i] = 3 << 10; i++; @@ -1365,7 +1396,10 @@ static void ivb_update_gamma(const struct intel_plane_state *plane_state) u16 gamma[18]; int i; - ivb_sprite_linear_gamma(gamma); + if (!ivb_need_sprite_gamma(plane_state)) + return; + + ivb_sprite_linear_gamma(plane_state, gamma); /* FIXME these register are single buffered :( */ for (i = 0; i < 16; i++) @@ -2507,6 +2541,8 @@ static bool snb_sprite_format_mod_supported(struct drm_plane *_plane, switch (format) { case DRM_FORMAT_XRGB: case DRM_FORMAT_XBGR: + case DRM_FORMAT_XRGB16161616F: + case DRM_FORMAT_XBGR16161616F: case DRM_FORMAT_YUYV: case DRM_FORMAT_YVYU: case DRM_FORMAT_UYVY: -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 19/19] drm/i915: Add support for half float framebuffers on snb sprites
From: Ville Syrjälä snb supports fp16 pixel formats on the sprite planes. Expose that capability. Nothing special needs to be done, it just works. v2: Rebase on top of icl fp16 Split snb+ sprite bits into a separate patch Reviewed-by: Maarten Lankhorst Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_sprite.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c index bd8ccc2de1da..d6c499900ab5 100644 --- a/drivers/gpu/drm/i915/display/intel_sprite.c +++ b/drivers/gpu/drm/i915/display/intel_sprite.c @@ -1622,6 +1622,12 @@ static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state, case DRM_FORMAT_XRGB: dvscntr |= DVS_FORMAT_RGBX888; break; + case DRM_FORMAT_XBGR16161616F: + dvscntr |= DVS_FORMAT_RGBX161616 | DVS_RGB_ORDER_XBGR; + break; + case DRM_FORMAT_XRGB16161616F: + dvscntr |= DVS_FORMAT_RGBX161616; + break; case DRM_FORMAT_YUYV: dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV; break; @@ -2330,8 +2336,10 @@ static const u64 i9xx_plane_format_modifiers[] = { }; static const u32 snb_plane_formats[] = { - DRM_FORMAT_XBGR, DRM_FORMAT_XRGB, + DRM_FORMAT_XBGR, + DRM_FORMAT_XRGB16161616F, + DRM_FORMAT_XBGR16161616F, DRM_FORMAT_YUYV, DRM_FORMAT_YVYU, DRM_FORMAT_UYVY, -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 00/14] drm: Try to fix encoder possible_clones/crtc
From: Ville Syrjälä Resurrecting my possible_clones/crtcs cleanup from ~1 year ago. Some prep work from the previous posting did get merged in the meantime. I have a feeling the WARNs from the new validation code may end up triggering on some drivers, but I don't imagine this stuff would get fixed without inflicting a bit of pain. Ville Syrjälä (14): drm: Include the encoder itself in possible_clones drm/gma500: Sanitize possible_clones drm/sti: Remove pointless casts drm/sti: Try to fix up the tvout possible clones drm/exynos: Use drm_encoder_mask() drm/imx: Remove the bogus possible_clones setup drm/i915: Polish possible_clones setup drm/i915: Populate possible_crtcs correctly drm/i915: Fix DP-MST crtc_mask drm/i915: Clean up encoder->crtc_mask setup drm/i915: Simplfy LVDS crtc_mask setup drm/i915: s/crtc_mask/pipe_mask/ drm: Validate encoder->possible_clones drm: Validate encoder->possible_crtcs drivers/gpu/drm/drm_encoder.c| 63 drivers/gpu/drm/exynos/exynos_drm_drv.c | 5 +- drivers/gpu/drm/gma500/framebuffer.c | 16 ++--- drivers/gpu/drm/gma500/mdfld_dsi_dpi.c | 4 +- drivers/gpu/drm/i915/display/icl_dsi.c | 2 +- drivers/gpu/drm/i915/display/intel_crt.c | 4 +- drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- drivers/gpu/drm/i915/display/intel_display.c | 28 ++--- drivers/gpu/drm/i915/display/intel_dp.c | 6 +- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/i915/display/intel_dvo.c | 2 +- drivers/gpu/drm/i915/display/intel_hdmi.c| 6 +- drivers/gpu/drm/i915/display/intel_lvds.c| 8 +-- drivers/gpu/drm/i915/display/intel_sdvo.c| 2 +- drivers/gpu/drm/i915/display/intel_tv.c | 3 +- drivers/gpu/drm/i915/display/vlv_dsi.c | 6 +- drivers/gpu/drm/i915/intel_drv.h | 5 +- drivers/gpu/drm/imx/imx-drm-core.c | 2 +- drivers/gpu/drm/sti/sti_tvout.c | 16 +++-- 19 files changed, 129 insertions(+), 53 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 01/14] drm: Include the encoder itself in possible_clones
From: Ville Syrjälä The docs say possible_clones should always include the encoder itself. Since most drivers don't want to deal with the complexities of cloning let's allow them to set possible_clones=0 and instead we'll fix that up in the core. We can't put this special case into drm_encoder_init() because drivers will have to fill up possible_clones after adding all the relevant encoders. Otherwise they wouldn't know the proper encoder indexes to use. So we'll just do it just before registering the encoders. TODO: Should we add the bit even if possible_clones was otherwise populated by the driver? Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_encoder.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 7fb47b7b8b44..e87e6fecc1fb 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -65,11 +65,26 @@ static const struct drm_prop_enum_list drm_encoder_enum_list[] = { { DRM_MODE_ENCODER_DPI, "DPI" }, }; +/* + * For some reason we want the encoder itself included in + * possible_clones. Make life easy for drivers by allowing them + * to leave possible_clones unset if no cloning is possible. + */ +static void fixup_possible_clones(struct drm_device *dev) +{ + struct drm_encoder *encoder; + + drm_for_each_encoder(encoder, dev) + encoder->possible_clones |= drm_encoder_mask(encoder); +} + int drm_encoder_register_all(struct drm_device *dev) { struct drm_encoder *encoder; int ret = 0; + fixup_possible_clones(dev); + drm_for_each_encoder(encoder, dev) { if (encoder->funcs->late_register) ret = encoder->funcs->late_register(encoder); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 02/14] drm/gma500: Sanitize possible_clones
From: Ville Syrjälä I doubt the DP+DP and SDVO+SDVO cloning works for this driver. i915 at least doesn't do those. Truthfully there could be some very specific circumstances where some of them would do doable, but genereally it's too much pain to deal with so we've chose not to bother. Let's use the same approach for gma500. Also the LVDS+LVDS and DSI+DSI cases probably don't really exist as there is one of each at most. This does mean we'll now leave possible_clones at 0 for these encoder types whereas previosuly we included the encoder itself in the bitmask. But that's fine as the core now treaks 0 as a special case and adds the encoder itself into the final bitmask reported to userspace. Cc: Patrik Jakobsson Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/gma500/framebuffer.c | 16 drivers/gpu/drm/gma500/mdfld_dsi_dpi.c | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 218f3bb15276..580a701fd91c 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -584,31 +584,31 @@ static void psb_setup_outputs(struct drm_device *dev) break; case INTEL_OUTPUT_SDVO: crtc_mask = dev_priv->ops->sdvo_mask; - clone_mask = (1 << INTEL_OUTPUT_SDVO); + clone_mask = 0; break; case INTEL_OUTPUT_LVDS: - crtc_mask = dev_priv->ops->lvds_mask; - clone_mask = (1 << INTEL_OUTPUT_LVDS); + crtc_mask = dev_priv->ops->lvds_mask; + clone_mask = 0; break; case INTEL_OUTPUT_MIPI: crtc_mask = (1 << 0); - clone_mask = (1 << INTEL_OUTPUT_MIPI); + clone_mask = 0; break; case INTEL_OUTPUT_MIPI2: crtc_mask = (1 << 2); - clone_mask = (1 << INTEL_OUTPUT_MIPI2); + clone_mask = 0; break; case INTEL_OUTPUT_HDMI: - crtc_mask = dev_priv->ops->hdmi_mask; + crtc_mask = dev_priv->ops->hdmi_mask; clone_mask = (1 << INTEL_OUTPUT_HDMI); break; case INTEL_OUTPUT_DISPLAYPORT: crtc_mask = (1 << 0) | (1 << 1); - clone_mask = (1 << INTEL_OUTPUT_DISPLAYPORT); + clone_mask = 0; break; case INTEL_OUTPUT_EDP: crtc_mask = (1 << 1); - clone_mask = (1 << INTEL_OUTPUT_EDP); + clone_mask = 0; } encoder->possible_crtcs = crtc_mask; encoder->possible_clones = diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c b/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c index d4c65f268922..187817e0c004 100644 --- a/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c +++ b/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c @@ -1006,10 +1006,10 @@ struct mdfld_dsi_encoder *mdfld_dsi_dpi_init(struct drm_device *dev, /*set possible crtcs and clones*/ if (dsi_connector->pipe) { encoder->possible_crtcs = (1 << 2); - encoder->possible_clones = (1 << 1); + encoder->possible_clones = 0; } else { encoder->possible_crtcs = (1 << 0); - encoder->possible_clones = (1 << 0); + encoder->possible_clones = 0; } dsi_connector->base.encoder = &dpi_output->base.base; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel