[Bug 209713] amdgpu drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_link_encoder.c:483 dcn10_get_dig_frontend+0x9e/0xc0 [amdgpu] when resuming from S3 state
https://bugzilla.kernel.org/show_bug.cgi?id=209713 Lahfa Samy (s...@lahfa.xyz) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |UNREPRODUCIBLE --- Comment #1 from Lahfa Samy (s...@lahfa.xyz) --- I cannot reproduce this call trace on the new kernel 5.9.1, so I could take that this issue was silently fixed ? I'll open the issue again if I see that the call trace shows up again someday. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm fixes part 2 for 5.10-rc1
Hi Linus, This should be the last round of things for rc1, a bunch of i915 fixes, some amdgpu, more font OOB fixes and one ttm fix just found reading code. Dave. drm-next-2020-10-23: drm fixes (round two) for 5.10-rc1 fbcon/fonts: - Two patches to prevent OOB access ttm: - fix for eviction value range check amdgpu: - Sienna Cichlid fixes - MST manager resource leak fix - GPU reset fix amdkfd: - Luxmark fix for Navi1x i915: - Tweak initial DPCD backlight.enabled value (Sean) - Initialize reserved MOCS indices (Ayaz) - Mark initial fb obj as WT on eLLC machines to avoid rcu lockup (Ville) - Support parsing of oversize batches (Chris) - Delay execlists processing for TGL (Chris) - Use the active reference on the vma during error capture (Chris) - Widen CSB pointer (Chris) - Wait for CSB entries on TGL (Chris) - Fix unwind for scratch page allocation (Chris) - Exclude low patches of stolen memory (Chris) - Force VT'd workarounds when running as a guest OS (Chris) - Drop runtime-pm assert from vpgu io accessors (Chris) The following changes since commit 40b99050455b9a6cb8faf15dcd41888312184720: Merge tag 'drm-intel-next-fixes-2020-10-15' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2020-10-19 09:21:59 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-next-2020-10-23 for you to fetch changes up to b45b6fbc671c60f56fd119c443e5570f83175928: Merge tag 'drm-intel-next-fixes-2020-10-22' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2020-10-23 09:52:18 +1000) drm fixes (round two) for 5.10-rc1 fbcon/fonts: - Two patches to prevent OOB access ttm: - fix for evicition value range check amdgpu: - Sienna Cichlid fixes - MST manager resource leak fix - GPU reset fix amdkfd: - Luxmark fix for Navi1x i915: - Tweak initial DPCD backlight.enabled value (Sean) - Initialize reserved MOCS indices (Ayaz) - Mark initial fb obj as WT on eLLC machines to avoid rcu lockup (Ville) - Support parsing of oversize batches (Chris) - Delay execlists processing for TGL (Chris) - Use the active reference on the vma during error capture (Chris) - Widen CSB pointer (Chris) - Wait for CSB entries on TGL (Chris) - Fix unwind for scratch page allocation (Chris) - Exclude low patches of stolen memory (Chris) - Force VT'd workarounds when running as a guest OS (Chris) - Drop runtime-pm assert from vpgu io accessors (Chris) Andrey Grodzovsky (3): drm/amd/display: Revert "drm/amd/display: Fix a list corruption" drm/amd/display: Avoid MST manager resource leak. drm/amd/psp: Fix sysfs: cannot create duplicate filename Ayaz A Siddiqui (1): drm/i915/gt: Initialize reserved and unspecified MOCS indices Chris Wilson (10): drm/i915/gem: Support parsing of oversize batches drm/i915/gt: Delay execlist processing for tgl drm/i915/gt: Undo forced context restores after trivial preemptions drm/i915: Use the active reference on the vma while capturing drm/i915/gt: Widen CSB pointer to u64 for the parsers drm/i915/gt: Wait for CSB entries on Tigerlake drm/i915/gt: Onion unwind for scratch page allocation failure drm/i915: Exclude low pages (128KiB) of stolen from use drm/i915: Force VT'd workarounds when running as a guest OS drm/i915: Drop runtime-pm assert from vgpu io accessors Dave Airlie (4): Merge tag 'drm-misc-next-fixes-2020-10-20' of git://anongit.freedesktop.org/drm/drm-misc into drm-next drm/ttm: fix eviction valuable range check. Merge tag 'amd-drm-fixes-5.10-2020-10-21' of git://people.freedesktop.org/~agd5f/linux into drm-next Merge tag 'drm-intel-next-fixes-2020-10-22' of git://anongit.freedesktop.org/drm/drm-intel into drm-next Evan Quan (1): drm/amdgpu: correct the gpu reset handling for job != NULL case Jay Cornwall (1): drm/amdkfd: Use same SQ prefetch setting as amdgpu John Clements (1): Revert drm/amdgpu: disable sienna chichlid UMC RAS Kenneth Feng (2): drm/amd/pm: fix pp_dpm_fclk drm/amd/pm: remove the average clock value in sysfs Kevin Wang (2): drm/amd/swsmu: add missing feature map for sienna_cichlid drm/amd/swsmu: correct wrong feature bit mapping Likun Gao (5): drm/amdgpu: add function to program pbb mode for sienna cichlid drm/amdgpu: add rlc iram and dram firmware support drm/amdgpu: update golden setting for sienna_cichlid drm/amd/pm: fix pcie information for sienna cichlid drm/amdgpu: correct the cu and rb info for sienna cichlid Peilin Ye (2): docs: fb: Add font_6x8 to available built-in fonts Fonts: Support FONT_EXTRA_WORDS macros for font_6x8 Sean Paul (1): drm/i915/dp: Tweak initial dpcd backlight.enabled value Ville Syrjälä (1): drm/i915: Mark ininitial fb obj as WT on eLLC machines to
[PATCH 1/3] drm/radeon: remove AGP support
> On Tue, May 12, 2020 at 4:16 AM Michel Dänzer wrote: > > > > On 2020-05-11 10:12 p.m., Alex Deucher wrote: > > > On Mon, May 11, 2020 at 1:17 PM Christian König > > > wrote: > > >> > > >> AGP is deprecated for 10+ years now and not used any more on modern > > >> hardware. > > >> > > >> Old hardware should continue to work in PCI mode. > > > > > > Might want to clarify that there is no loss of functionality here. > > > Something like: > > > > > > "There is no loss of functionality here. GPUs will continue to > > > function. This just drops the use of the AGP MMU in the chipset in > > > favor of the MMU on the device which has proven to be much more > > > reliable. Due to its unreliability, AGP support has been disabled on > > > PowerPC for years already so there is no change on PowerPC." > > > > There's a difference between something being disabled by default or not > > being available at all. We may decide it's worth it anyway, but let's do > > it based on facts. > > > > I didn't mean to imply that AGP GART support was already removed. But > for the vast majority of users the end result is the same. If you > knew enough re-enable AGP GART, you probably wouldn't have been as > confused about what this patch set does either. To reiterate, this > patch set does not remove support for AGP cards, it only removes the > support for AGP GART. The cards will still be functional using the > device GART. There may be performance tradeoffs there in some cases. > > Alex Back in the fglrx days, the fglrxconfig utility proposed: == Advanced OS Settings == External AGPGART module: It is possible (but not recommended) to turn off the usage of built-in agp support of the provided fglrx kernel module and use the external AGP GART module (agpgart.o) of the Linux kernel. If you want to use the external module then ensure that it loads prior to the drivers full startup. In order to manually load the external agpgart module execute this on the commandline (as root): /sbin/insmod agpgart or alternatively configure your system to auto load the module. Do you want to use the external AGP GART module (y/n)? [n] By "built-in agp support of the provided fglrx kernel module", was fglrxconfig referring to the "device GART"? So the device-side IOMMU allowing to "work in PCI mode", somewhat dubbed as "PCI GART" (even for AGP graphics adapters?!??) in some messages here? If so, it is noteworthy that the default choice was not to use the external AGP GART (resulting in Option "UseInternalAGPGART" "yes" in /etc/X11/XF86Config), thus leveraging the device GART, i.e. what Christian proposal is all about. Unless I still mix everything. Émeric ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: amdgpu: Manual Card Configuration Change
‐‐‐ Original Message ‐‐‐ On Wednesday, October 21, 2020 2:44 PM, Alex Deucher wrote: > On Mon, Oct 19, 2020 at 8:53 PM Josh Fuhs joshua.f...@pm.me wrote: > > > Thanks. I tried 5.9.1 and I think there's still a problem, or at least > > something different. > > Using the same configuration script, I noticed that my cards are running a > > lot hotter. For example, here's total power consumption of a two-card > > system with two different kernels: > > > > 5.8.14: 460W > > 5.9.1: 560+W > > > > > > Memory and system clocks are initially set the same on all cards in all > > cases. > > Can you bisect? > I assume this means using git bisect to narrow down the commit that introduced the effect. I'm not set up for kernel builds. Is there a guide? Josh ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-next-fixes
Hi Dave and Daniel, Here is probably the last drm-intel-next-fixes before -rc1. This includes a few patches from dinq and a bunch from drm-intel-gt-next. drm-intel-next-fixes-2020-10-22: - Tweak initia DPCD backlight.enabled value (Sean) - Initialize reserved MOCS indices (Ayaz) - Mark initial fb obj as WT on eLLC machines to avoid rcu lockup (Ville) - Support parsing of oversize batches (Chris) - Delay execlists processing for TGL (Chris) - Use the active reference on the vma during error capture (Chris) - Widen CSB pointer (Chris) - Wait for CSB entries on TGL (Chris) - Fix unwind for scratch page allocation (Chris) - Exclude low patches of stolen memory (Chris) - Force VT'd workarounds when running as a guest OS (Chris) - Drop runtime-pm assert from vpgu io accessors (Chris) The following changes since commit 214bba50616f65264dfc30d095daef3ab7500f52: drm/i915: Set all unused color plane offsets to ~0xfff again (2020-10-12 14:23:22 -0400) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-fixes-2020-10-22 for you to fetch changes up to 5c6c13cd1102caf92d006a3cf4591c0229019daf: drm/i915: Drop runtime-pm assert from vgpu io accessors (2020-10-21 08:32:32 -0400) - Tweak initia DPCD backlight.enabled value (Sean) - Initialize reserved MOCS indices (Ayaz) - Mark initial fb obj as WT on eLLC machines to avoid rcu lockup (Ville) - Support parsing of oversize batches (Chris) - Delay execlists processing for TGL (Chris) - Use the active reference on the vma during error capture (Chris) - Widen CSB pointer (Chris) - Wait for CSB entries on TGL (Chris) - Fix unwind for scratch page allocation (Chris) - Exclude low patches of stolen memory (Chris) - Force VT'd workarounds when running as a guest OS (Chris) - Drop runtime-pm assert from vpgu io accessors (Chris) Ayaz A Siddiqui (1): drm/i915/gt: Initialize reserved and unspecified MOCS indices Chris Wilson (10): drm/i915/gem: Support parsing of oversize batches drm/i915/gt: Delay execlist processing for tgl drm/i915/gt: Undo forced context restores after trivial preemptions drm/i915: Use the active reference on the vma while capturing drm/i915/gt: Widen CSB pointer to u64 for the parsers drm/i915/gt: Wait for CSB entries on Tigerlake drm/i915/gt: Onion unwind for scratch page allocation failure drm/i915: Exclude low pages (128KiB) of stolen from use drm/i915: Force VT'd workarounds when running as a guest OS drm/i915: Drop runtime-pm assert from vgpu io accessors Sean Paul (1): drm/i915/dp: Tweak initial dpcd backlight.enabled value Ville Syrjälä (1): drm/i915: Mark ininitial fb obj as WT on eLLC machines to avoid rcu lockup during fbdev init drivers/gpu/drm/i915/Kconfig.debug | 1 + drivers/gpu/drm/i915/display/intel_display.c | 8 + .../gpu/drm/i915/display/intel_dp_aux_backlight.c | 31 ++-- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 6 +- drivers/gpu/drm/i915/gem/i915_gem_stolen.h | 2 + drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 18 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 3 +- drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 +- drivers/gpu/drm/i915/gt/intel_lrc.c| 58 +++--- drivers/gpu/drm/i915/gt/intel_mocs.c | 16 +- drivers/gpu/drm/i915/gt/selftest_reset.c | 196 + drivers/gpu/drm/i915/i915_drv.h| 6 +- drivers/gpu/drm/i915/i915_gpu_error.c | 3 +- drivers/gpu/drm/i915/intel_uncore.c| 27 ++- 15 files changed, 334 insertions(+), 53 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/modes: Switch to 64bit maths to avoid integer overflow
From: Ville Syrjälä The new >8k CEA modes have dotclocks reaching 5.94 GHz, which means our clock*1000 will now overflow the 32bit unsigned integer. Switch to 64bit maths to avoid it. Cc: sta...@vger.kernel.org Reported-by: Randy Dunlap Signed-off-by: Ville Syrjälä --- An interesting question how many other place might suffer from similar overflows. I think i915 should be mostly OK. The one place I know we use Hz instead kHz is the hsw DPLL code, which I would prefer we also change to use kHz. The other concern is whether we have any potential overflows before we check this against the platform's max dotclock. I do have this unreviewed igt series https://patchwork.freedesktop.org/series/69531/ which extends the current testing with some other forms of invalid modes. Could probably extend that with a mode.clock=INT_MAX test to see if anything else might trip up. No idea about other drivers. drivers/gpu/drm/drm_modes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 501b4fe55a3d..511cde5c7fa6 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -762,7 +762,7 @@ int drm_mode_vrefresh(const struct drm_display_mode *mode) if (mode->htotal == 0 || mode->vtotal == 0) return 0; - num = mode->clock * 1000; + num = mode->clock; den = mode->htotal * mode->vtotal; if (mode->flags & DRM_MODE_FLAG_INTERLACE) @@ -772,7 +772,7 @@ int drm_mode_vrefresh(const struct drm_display_mode *mode) if (mode->vscan > 1) den *= mode->vscan; - return DIV_ROUND_CLOSEST(num, den); + return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(num, 1000), den); } EXPORT_SYMBOL(drm_mode_vrefresh); -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/3] drm/atomic-helpers: remove legacy_cursor_update hacks
On Thu, Oct 22, 2020 at 7:22 PM Rob Clark wrote:
>
> On Thu, Oct 22, 2020 at 10:02 AM Rob Clark wrote:
> >
> > On Wed, Oct 21, 2020 at 9:32 AM Daniel Vetter
> > wrote:
> > >
> > > The stuff never really worked, and leads to lots of fun because it
> > > out-of-order frees atomic states. Which upsets KASAN, among other
> > > things.
> > >
> > > For async updates we now have a more solid solution with the
> > > ->atomic_async_check and ->atomic_async_commit hooks. Support for that
> > > for msm and vc4 landed. nouveau and i915 have their own commit
> > > routines, doing something similar.
> > >
> > > For everyone else it's probably better to remove the use-after-free
> > > bug, and encourage folks to use the async support instead. The
> > > affected drivers which register a legacy cursor plane and don't either
> > > use the new async stuff or their own commit routine are: amdgpu,
> > > atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx.
> > >
> > > Inspired by an amdgpu bug report.
> > >
> > > v2: Drop RFC, I think with amdgpu converted over to use
> > > atomic_async_check/commit done in
> > >
> > > commit 674e78acae0dfb4beb56132e41cbae5b60f7d662
> > > Author: Nicholas Kazlauskas
> > > Date: Wed Dec 5 14:59:07 2018 -0500
> > >
> > > drm/amd/display: Add fast path for cursor plane updates
> > >
> > > we don't have any driver anymore where we have userspace expecting
> > > solid legacy cursor support _and_ they are using the atomic helpers in
> > > their fully glory. So we can retire this.
> > >
> > > References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
> > > Cc: mikita.lip...@amd.com
> > > Cc: Michel Dänzer
> > > Cc: harry.wentl...@amd.com
> > > Signed-off-by: Daniel Vetter
> >
> > This *completely* destroys fps when there is cursor movement, it would
> > be a pretty bad regression, so nak
>
> Which I *guess* is due to dpu not wiring up the plane->async_* funcs,
> effectively making cursor updates synchronous.. but it will take some
> time to sort out :-(
Does something like the below (not even compile tested) get dpu back in order?
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 561bfa48841c..ec8b4f74da49 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -215,6 +215,8 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state)
/* async updates are limited to single-crtc updates: */
WARN_ON(crtc_mask != drm_crtc_mask(async_crtc));
+ complete_all(async_crtc->state->flip_done);
+
/*
* Start timer if we don't already have an update pending
* on this crtc:
That way we could perhaps still move ahead with removing the hacks
from shared helpers, and msm-dpu can keep doing what it does. The
other hunk is in a function that dpu code doesn't even use, so can't
see how that would change anything.
-Daniel
>
> > BR,
> > -R
> >
> > > ---
> > > drivers/gpu/drm/drm_atomic_helper.c | 13 -
> > > 1 file changed, 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index a7bcb4b4586c..549a31e6042c 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -1481,13 +1481,6 @@ drm_atomic_helper_wait_for_vblanks(struct
> > > drm_device *dev,
> > > int i, ret;
> > > unsigned crtc_mask = 0;
> > >
> > > -/*
> > > - * Legacy cursor ioctls are completely unsynced, and userspace
> > > - * relies on that (by doing tons of cursor updates).
> > > - */
> > > - if (old_state->legacy_cursor_update)
> > > - return;
> > > -
> > > for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
> > > new_crtc_state, i) {
> > > if (!new_crtc_state->active)
> > > continue;
> > > @@ -2106,12 +2099,6 @@ int drm_atomic_helper_setup_commit(struct
> > > drm_atomic_state *state,
> > > continue;
> > > }
> > >
> > > - /* Legacy cursor updates are fully unsynced. */
> > > - if (state->legacy_cursor_update) {
> > > - complete_all(>flip_done);
> > > - continue;
> > > - }
> > > -
> > > if (!new_crtc_state->event) {
> > > commit->event = kzalloc(sizeof(*commit->event),
> > > GFP_KERNEL);
> > > --
> > > 2.28.0
> > >
> > > ___
> > > Intel-gfx mailing list
> > > intel-...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
Re: [PATCH] drm/edid: Fix uninitialized variable in drm_cvt_modes()
On Thu, Oct 22, 2020 at 12:55 PM Lyude Paul wrote:
>
> Noticed this when trying to compile with -Wall on a kernel fork. We
> potentially
> don't set width here, which causes the compiler to complain about width
> potentially being uninitialized in drm_cvt_modes(). So, let's fix that.
>
> Signed-off-by: Lyude Paul
>
> Cc: # v5.9+
> Fixes: 3f649ab728cd ("treewide: Remove uninitialized_var() usage")
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/drm_edid.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 631125b46e04..2da158ffed8e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3094,6 +3094,7 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
>
> for (i = 0; i < 4; i++) {
> int width, height;
> + u8 cvt_aspect_ratio;
>
> cvt = &(timing->data.other_data.data.cvt[i]);
>
> @@ -3101,7 +3102,8 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
> continue;
>
> height = (cvt->code[0] + ((cvt->code[1] & 0xf0) << 4) + 1) *
> 2;
> - switch (cvt->code[1] & 0x0c) {
> + cvt_aspect_ratio = cvt->code[1] & 0x0c;
> + switch (cvt_aspect_ratio) {
> case 0x00:
> width = height * 4 / 3;
> break;
> @@ -3114,6 +3116,10 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
> case 0x0c:
> width = height * 15 / 9;
> break;
> + default:
What value would cvt->code[1] have such that this gets hit?
Or is this a "compiler is broken, so let's add more code" situation?
If so, perhaps the code added could just be enough to silence the
compiler (unreachable, etc)?
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/3] drm/atomic-helpers: remove legacy_cursor_update hacks
On Thu, Oct 22, 2020 at 10:02 AM Rob Clark wrote:
>
> On Wed, Oct 21, 2020 at 9:32 AM Daniel Vetter wrote:
> >
> > The stuff never really worked, and leads to lots of fun because it
> > out-of-order frees atomic states. Which upsets KASAN, among other
> > things.
> >
> > For async updates we now have a more solid solution with the
> > ->atomic_async_check and ->atomic_async_commit hooks. Support for that
> > for msm and vc4 landed. nouveau and i915 have their own commit
> > routines, doing something similar.
> >
> > For everyone else it's probably better to remove the use-after-free
> > bug, and encourage folks to use the async support instead. The
> > affected drivers which register a legacy cursor plane and don't either
> > use the new async stuff or their own commit routine are: amdgpu,
> > atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx.
> >
> > Inspired by an amdgpu bug report.
> >
> > v2: Drop RFC, I think with amdgpu converted over to use
> > atomic_async_check/commit done in
> >
> > commit 674e78acae0dfb4beb56132e41cbae5b60f7d662
> > Author: Nicholas Kazlauskas
> > Date: Wed Dec 5 14:59:07 2018 -0500
> >
> > drm/amd/display: Add fast path for cursor plane updates
> >
> > we don't have any driver anymore where we have userspace expecting
> > solid legacy cursor support _and_ they are using the atomic helpers in
> > their fully glory. So we can retire this.
> >
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
> > Cc: mikita.lip...@amd.com
> > Cc: Michel Dänzer
> > Cc: harry.wentl...@amd.com
> > Signed-off-by: Daniel Vetter
>
> This *completely* destroys fps when there is cursor movement, it would
> be a pretty bad regression, so nak
Which I *guess* is due to dpu not wiring up the plane->async_* funcs,
effectively making cursor updates synchronous.. but it will take some
time to sort out :-(
> BR,
> -R
>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 13 -
> > 1 file changed, 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index a7bcb4b4586c..549a31e6042c 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1481,13 +1481,6 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device
> > *dev,
> > int i, ret;
> > unsigned crtc_mask = 0;
> >
> > -/*
> > - * Legacy cursor ioctls are completely unsynced, and userspace
> > - * relies on that (by doing tons of cursor updates).
> > - */
> > - if (old_state->legacy_cursor_update)
> > - return;
> > -
> > for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
> > new_crtc_state, i) {
> > if (!new_crtc_state->active)
> > continue;
> > @@ -2106,12 +2099,6 @@ int drm_atomic_helper_setup_commit(struct
> > drm_atomic_state *state,
> > continue;
> > }
> >
> > - /* Legacy cursor updates are fully unsynced. */
> > - if (state->legacy_cursor_update) {
> > - complete_all(>flip_done);
> > - continue;
> > - }
> > -
> > if (!new_crtc_state->event) {
> > commit->event = kzalloc(sizeof(*commit->event),
> > GFP_KERNEL);
> > --
> > 2.28.0
> >
> > ___
> > Intel-gfx mailing list
> > intel-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/3] drm/atomic-helpers: remove legacy_cursor_update hacks
On Wed, Oct 21, 2020 at 9:32 AM Daniel Vetter wrote:
>
> The stuff never really worked, and leads to lots of fun because it
> out-of-order frees atomic states. Which upsets KASAN, among other
> things.
>
> For async updates we now have a more solid solution with the
> ->atomic_async_check and ->atomic_async_commit hooks. Support for that
> for msm and vc4 landed. nouveau and i915 have their own commit
> routines, doing something similar.
>
> For everyone else it's probably better to remove the use-after-free
> bug, and encourage folks to use the async support instead. The
> affected drivers which register a legacy cursor plane and don't either
> use the new async stuff or their own commit routine are: amdgpu,
> atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx.
>
> Inspired by an amdgpu bug report.
>
> v2: Drop RFC, I think with amdgpu converted over to use
> atomic_async_check/commit done in
>
> commit 674e78acae0dfb4beb56132e41cbae5b60f7d662
> Author: Nicholas Kazlauskas
> Date: Wed Dec 5 14:59:07 2018 -0500
>
> drm/amd/display: Add fast path for cursor plane updates
>
> we don't have any driver anymore where we have userspace expecting
> solid legacy cursor support _and_ they are using the atomic helpers in
> their fully glory. So we can retire this.
>
> References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
> Cc: mikita.lip...@amd.com
> Cc: Michel Dänzer
> Cc: harry.wentl...@amd.com
> Signed-off-by: Daniel Vetter
This *completely* destroys fps when there is cursor movement, it would
be a pretty bad regression, so nak
BR,
-R
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 13 -
> 1 file changed, 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index a7bcb4b4586c..549a31e6042c 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1481,13 +1481,6 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device
> *dev,
> int i, ret;
> unsigned crtc_mask = 0;
>
> -/*
> - * Legacy cursor ioctls are completely unsynced, and userspace
> - * relies on that (by doing tons of cursor updates).
> - */
> - if (old_state->legacy_cursor_update)
> - return;
> -
> for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
> new_crtc_state, i) {
> if (!new_crtc_state->active)
> continue;
> @@ -2106,12 +2099,6 @@ int drm_atomic_helper_setup_commit(struct
> drm_atomic_state *state,
> continue;
> }
>
> - /* Legacy cursor updates are fully unsynced. */
> - if (state->legacy_cursor_update) {
> - complete_all(>flip_done);
> - continue;
> - }
> -
> if (!new_crtc_state->event) {
> commit->event = kzalloc(sizeof(*commit->event),
> GFP_KERNEL);
> --
> 2.28.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/edid: Fix uninitialized variable in drm_cvt_modes()
Noticed this when trying to compile with -Wall on a kernel fork. We potentially
don't set width here, which causes the compiler to complain about width
potentially being uninitialized in drm_cvt_modes(). So, let's fix that.
Signed-off-by: Lyude Paul
Cc: # v5.9+
Fixes: 3f649ab728cd ("treewide: Remove uninitialized_var() usage")
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/drm_edid.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 631125b46e04..2da158ffed8e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3094,6 +3094,7 @@ static int drm_cvt_modes(struct drm_connector *connector,
for (i = 0; i < 4; i++) {
int width, height;
+ u8 cvt_aspect_ratio;
cvt = &(timing->data.other_data.data.cvt[i]);
@@ -3101,7 +3102,8 @@ static int drm_cvt_modes(struct drm_connector *connector,
continue;
height = (cvt->code[0] + ((cvt->code[1] & 0xf0) << 4) + 1) * 2;
- switch (cvt->code[1] & 0x0c) {
+ cvt_aspect_ratio = cvt->code[1] & 0x0c;
+ switch (cvt_aspect_ratio) {
case 0x00:
width = height * 4 / 3;
break;
@@ -3114,6 +3116,10 @@ static int drm_cvt_modes(struct drm_connector *connector,
case 0x0c:
width = height * 15 / 9;
break;
+ default:
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] unknown CVT aspect
ratio %x\n",
+ connector->base.id, connector->name,
cvt_aspect_ratio);
+ continue;
}
for (j = 1; j < 5; j++) {
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Outreachy kernel][PATCH 0/5] drm/amdgpu: Replace snprintf() with sysfs_emit
On Thu, Oct 22, 2020 at 07:07:50PM +0530, Sumera Priyadarsini wrote: > Using snprintf() for show() methods holds the risk of buffer overrun > as snprintf() does not know the PAGE_SIZE maximum of the temporary > buffer used to output sysfs content. > > This patchset is a series of Coccinelle cleanups across the staging > directory to convert snprintf with scnprintf in the relevant files. I think you need to edit your template here since this is now drivers/gpu, not staging :-) -Daniel > > Sumera Priyadarsini (5): > gpu: drm: amdgpu: Replace snprintf() with sysfs_emit() > gpu: drm: amdgpu: Replace snprintf() with sysfs_emit() > gpu: drm: amdgpu: Replace snprintf() with sysfs_emit() > gpu: drm: amdgpu: Replace snprintf() with sysfs_emit() > gpu: drm: amdgpu: Replace snprintf() with sysfs_emit() > > drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 > drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 4 ++-- > 5 files changed, 10 insertions(+), 10 deletions(-) > > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/atomic-helpers: remove legacy_cursor_update hacks
On Thu, Oct 22, 2020 at 09:36:23AM -0400, Kazlauskas, Nicholas wrote:
> On 2020-10-21 12:32 p.m., Daniel Vetter wrote:
> > The stuff never really worked, and leads to lots of fun because it
> > out-of-order frees atomic states. Which upsets KASAN, among other
> > things.
> >
> > For async updates we now have a more solid solution with the
> > ->atomic_async_check and ->atomic_async_commit hooks. Support for that
> > for msm and vc4 landed. nouveau and i915 have their own commit
> > routines, doing something similar.
> >
> > For everyone else it's probably better to remove the use-after-free
> > bug, and encourage folks to use the async support instead. The
> > affected drivers which register a legacy cursor plane and don't either
> > use the new async stuff or their own commit routine are: amdgpu,
> > atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx.
> >
> > Inspired by an amdgpu bug report.
> >
> > v2: Drop RFC, I think with amdgpu converted over to use
> > atomic_async_check/commit done in
> >
> > commit 674e78acae0dfb4beb56132e41cbae5b60f7d662
> > Author: Nicholas Kazlauskas
> > Date: Wed Dec 5 14:59:07 2018 -0500
> >
> > drm/amd/display: Add fast path for cursor plane updates
> >
> > we don't have any driver anymore where we have userspace expecting
> > solid legacy cursor support _and_ they are using the atomic helpers in
> > their fully glory. So we can retire this.
> >
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
> > Cc: mikita.lip...@amd.com
> > Cc: Michel Dänzer
> > Cc: harry.wentl...@amd.com
> > Signed-off-by: Daniel Vetter
>
> I'm fine with the idea but it looks like we need modification to amdgpu to
> not break anything:
>
> if (state->legacy_cursor_update) {
> /* ... */
> state->async_update =
> !drm_atomic_helper_async_check(dev, state);
>
>
> We only check async update for legacy_cursor_updates here which won't cover
> the atomic path. I think it's safe to drop the check here but that should
> probably be done before or as part of this series.
This part is fine, you're essentially duplicating what the helpers are
doing too. I'm not sure whether we should lift this to core atomic
semantics or something else, but should be all ok as-is. Might still be
good to test this in case something isn't 100% complete and amdgpu atomic
commit still relies on legacy_cursor_update semantics somewhere.
But after this patch your atomic code and atomic helpers check/commit
functions match (I think), so we /should/ be good.
Cheers, Daniel
>
> Regards,
> Nicholas Kazlauskas
>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 13 -
> > 1 file changed, 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index a7bcb4b4586c..549a31e6042c 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1481,13 +1481,6 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device
> > *dev,
> > int i, ret;
> > unsigned crtc_mask = 0;
> > -/*
> > - * Legacy cursor ioctls are completely unsynced, and userspace
> > - * relies on that (by doing tons of cursor updates).
> > - */
> > - if (old_state->legacy_cursor_update)
> > - return;
> > -
> > for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
> > new_crtc_state, i) {
> > if (!new_crtc_state->active)
> > continue;
> > @@ -2106,12 +2099,6 @@ int drm_atomic_helper_setup_commit(struct
> > drm_atomic_state *state,
> > continue;
> > }
> > - /* Legacy cursor updates are fully unsynced. */
> > - if (state->legacy_cursor_update) {
> > - complete_all(>flip_done);
> > - continue;
> > - }
> > -
> > if (!new_crtc_state->event) {
> > commit->event = kzalloc(sizeof(*commit->event),
> > GFP_KERNEL);
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Outreachy kernel] [PATCH 4/5] gpu: drm: amdgpu: Replace snprintf() with sysfs_emit()
On Thu, Oct 22, 2020 at 07:17:56PM +0530, Sumera Priyadarsini wrote: > Using snprintf() for show() methods holds the risk of buffer overrun > as snprintf() does not know the PAGE_SIZE maximum of the temporary > buffer used to output sysfs content. > > Modify amdgpu_psp.c to use sysfs_emit() instead which knows the > size of the temporary buffer. > > Issue found with Coccinelle. > > Signed-off-by: Sumera Priyadarsini > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index d6c38e24f130..4d1d1e1b005d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -2621,7 +2621,7 @@ static ssize_t psp_usbc_pd_fw_sysfs_read(struct device > *dev, > return ret; > } > > - return snprintf(buf, PAGE_SIZE, "%x\n", fw_ver); > + return sysfs_emit(buf, PAGE_SIZE, "%x\n", fw_ver); Did you build this code? I don't think it is correct... thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/atomic-helpers: remove legacy_cursor_update hacks
On 2020-10-21 12:32 p.m., Daniel Vetter wrote:
The stuff never really worked, and leads to lots of fun because it
out-of-order frees atomic states. Which upsets KASAN, among other
things.
For async updates we now have a more solid solution with the
->atomic_async_check and ->atomic_async_commit hooks. Support for that
for msm and vc4 landed. nouveau and i915 have their own commit
routines, doing something similar.
For everyone else it's probably better to remove the use-after-free
bug, and encourage folks to use the async support instead. The
affected drivers which register a legacy cursor plane and don't either
use the new async stuff or their own commit routine are: amdgpu,
atmel, mediatek, qxl, rockchip, sti, sun4i, tegra, virtio, and vmwgfx.
Inspired by an amdgpu bug report.
v2: Drop RFC, I think with amdgpu converted over to use
atomic_async_check/commit done in
commit 674e78acae0dfb4beb56132e41cbae5b60f7d662
Author: Nicholas Kazlauskas
Date: Wed Dec 5 14:59:07 2018 -0500
drm/amd/display: Add fast path for cursor plane updates
we don't have any driver anymore where we have userspace expecting
solid legacy cursor support _and_ they are using the atomic helpers in
their fully glory. So we can retire this.
References: https://bugzilla.kernel.org/show_bug.cgi?id=199425
Cc: mikita.lip...@amd.com
Cc: Michel Dänzer
Cc: harry.wentl...@amd.com
Signed-off-by: Daniel Vetter
I'm fine with the idea but it looks like we need modification to amdgpu
to not break anything:
if (state->legacy_cursor_update) {
/* ... */
state->async_update =
!drm_atomic_helper_async_check(dev, state);
We only check async update for legacy_cursor_updates here which won't
cover the atomic path. I think it's safe to drop the check here but that
should probably be done before or as part of this series.
Regards,
Nicholas Kazlauskas
---
drivers/gpu/drm/drm_atomic_helper.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index a7bcb4b4586c..549a31e6042c 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1481,13 +1481,6 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device
*dev,
int i, ret;
unsigned crtc_mask = 0;
- /*
- * Legacy cursor ioctls are completely unsynced, and userspace
- * relies on that (by doing tons of cursor updates).
- */
- if (old_state->legacy_cursor_update)
- return;
-
for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
new_crtc_state, i) {
if (!new_crtc_state->active)
continue;
@@ -2106,12 +2099,6 @@ int drm_atomic_helper_setup_commit(struct
drm_atomic_state *state,
continue;
}
- /* Legacy cursor updates are fully unsynced. */
- if (state->legacy_cursor_update) {
- complete_all(>flip_done);
- continue;
- }
-
if (!new_crtc_state->event) {
commit->event = kzalloc(sizeof(*commit->event),
GFP_KERNEL);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/3] drm/panel: mantix panel reset fixes
On Tue, Oct 20, 2020 at 01:57:11PM +0200, Guido Günther wrote:
> Hi Daniel, Sam,
> On Mon, Oct 19, 2020 at 05:44:37PM +0200, Daniel Vetter wrote:
> > On Sat, Oct 17, 2020 at 12:47:36PM +0200, Sam Ravnborg wrote:
> > > Hi Guido.
> > >
> > > On Sat, Oct 17, 2020 at 11:13:07AM +0200, Guido Günther wrote:
> > > > Hi Sam,
> > > > On Fri, Oct 16, 2020 at 04:29:16PM +0200, Sam Ravnborg wrote:
> > > > > Hi Guido.
> > > > > On Tue, Oct 13, 2020 at 12:32:45PM +0200, Guido Günther wrote:
> > > > [..snip..]
> > > > > >
> > > > > > Changes from v1:
> > > > > > - As per review comments by Fabio Estevam
> > > > > >
> > > > > > https://lore.kernel.org/dri-devel/CAOMZO5B5ECcConvKej=rcaf8wvoxgq7nuzkj-ad0asaozuq...@mail.gmail.com/
> > > > > >- Fix typo in commit messages
> > > > > > - As per review comments by Rob Herring
> > > > > >https://lore.kernel.org/dri-devel/20200929174624.GA832332@bogus/
> > > > > >- Don't use an array of reset lines
> > > > > >
> > > > > > Guido Günther (3):
> > > > > > drm/panel: mantix: Don't dereference NULL mode
> > > > > > drm/panel: mantix: Fix panel reset
> > > > > > dt-binding: display: Require two resets on mantix panel
> > > > >
> > > > > All applied to drm-misc-next and pushed out.
> > > > > And then I remembered you had commit right - sigh.
> > > >
> > > > Thanks! Is there any special care needed to get that into 5.10? The
> > > > driver landed there in 72967d5616d3f0c714f8eb6c4e258179a9031c45.
> > >
> > > As the patches was applied to drm-misc-next the easiet path would
> > > be to cherry-pick them and apply to drm-misc-fixes.
> > > dim has cherry-pick support - try to use it rahter than doing it by
> > > hand.
> >
> > drm-misc-next-fixes while we're between freeze and merge window end:
> >
> > https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-misc.html#where-do-i-apply-my-patch
>
> Great. Thanks for the pointer, that works. Now i get:
>
> $ ../maintainer-tools/dim push --dry-run
> dim: 3532f0117258 ("dt-binding: display: Require two resets on mantix
> panel"): mandatory review missing.
> dim: c90f95ad6d05 ("drm/panel: mantix: Fix panel reset"): mandatory review
> missing.
> dim: 8b557f793e69 ("drm/panel: mantix: Don't dereference NULL mode"):
> mandatory review missing.
> dim: ERROR: issues in commits detected, aborting
Ah yes, if author != committer then dim assumes there has been some
oversight and review.
But when you cherry-pick over then it's again author == committer and in
that case dim is looking for an a-b or r-b line, hence why the cherry-pick
fails.
For a bit of context.
-Daniel
> and in fact there's only Signed-off-by's on it:
>
> Fixes: 72967d5616d3 ("drm/panel: Add panel driver for the Mantix
> MLAF057WE51-X DSI panel")
> Signed-off-by: Guido Günther
> Signed-off-by: Sam Ravnborg
> Link:
> https://patchwork.freedesktop.org/patch/msgid/ba71a8ab010d263a8058dd4f711e3bcd95877bf2.1602584953.git@sigxcpu.org
>
> Sam, I assume your Signed-off-by's should have been Reviewed-by ?
> O.k. to fix that up in the commit message before pushing to
> drm-misc-next?
>
> Cheers,
> -- Guido
>
> >
> > Cheers, Daniel
> >
> > >
> >
> > > When you apply to drm-misc-fixes include a Fixes: tag so the tooling
> > > will pick the patches automagically.
> > >
> > > In hindsight the patches should have carried a Fixes: tag from a start
> > > and should have been applied to drm-misc-fixes from a start too.
> > >
> > > I have done something like above once or twice but anyway reach out if
> > > you have questions. Or ask at #dri-devel.
> > >
> > > Sam
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> >
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 12/16] PCI: Obey iomem restrictions for procfs mmap
On Thu, Oct 22, 2020 at 1:43 PM Jason Gunthorpe wrote: > > On Thu, Oct 22, 2020 at 09:00:44AM +0200, Daniel Vetter wrote: > > On Thu, Oct 22, 2020 at 1:20 AM Jason Gunthorpe wrote: > > > > > > On Wed, Oct 21, 2020 at 09:24:08PM +0200, Daniel Vetter wrote: > > > > On Wed, Oct 21, 2020 at 6:37 PM Jason Gunthorpe wrote: > > > > > > > > > > On Wed, Oct 21, 2020 at 05:54:54PM +0200, Daniel Vetter wrote: > > > > > > > > > > > The trouble is that io_remap_pfn adjust vma->pgoff, so we'd need to > > > > > > split that. So ideally ->mmap would never set up any ptes. > > > > > > > > > > /dev/mem makes pgoff == pfn so it doesn't get changed by remap. > > > > > > > > > > pgoff doesn't get touched for MAP_SHARED either, so there are other > > > > > users that could work like this - eg anyone mmaping IO memory is > > > > > probably OK. > > > > > > > > I was more generally thinking for io_remap_pfn_users because of the > > > > mkwrite use-case we might have in fbdev emulation in drm. > > > > > > You have a use case for MAP_PRIVATE and io_remap_pfn_range()?? > > > > Uh no :-) > > So it is fine, the pgoff mangling only happens for MAP_PRIVATE Ah right I got confused, thanks for clarifying. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: Store USB device in struct drm_device
Hi, On 10/22/20 12:57 PM, Thomas Zimmermann wrote: > Hi Hans > > On 22.10.20 12:17, Hans de Goede wrote: >> Hi, >> >> On 10/22/20 11:30 AM, Thomas Zimmermann wrote: >>> Hi >>> >>> On 22.10.20 11:20, Hans de Goede wrote: Hi, On 10/21/20 3:07 PM, Thomas Zimmermann wrote: > The drivers gm12u320 and udl operate on USB devices. They leave the > PCI device in struct drm_device empty and store the USB device in their > own driver structure. > > Fix this special case and save a few bytes by putting the USB device > into an anonymous union with the PCI data. It's expected that DRM > core and helpers only touch the PCI-device field for actual PCI devices. > > Thomas Zimmermann (3): > drm: Add reference to USB device to struct drm_device > drm/tiny/gm12u320: Store USB device in struct drm_device.udev > drm/udl: Store USB device in struct drm_device.udev This series looks good to me: Reviewed-by: Hans de Goede >>> >>> Thanks a lot. Following Daniel's request, I'll drop patch 1 and instead >>> do an upcast from drm_device.dev to the USB device structure. The driver >>> patches 2 and 3 will be slightly different. Unless you object, I''ll >>> take the r-b into the new patches. >> >> I somehow missed Daniel's reply about this. >> >> With that said, hmm that is going to be an interesting up-cast, at least >> for the gm12u320, that is going to look something like this: >> >> struct usb_device *udev = >> interface_to_usbdev(to_usb_interface(drm_dev->dev)); >> >> (I wrote drm_dev instead of dev to make it more clear what is going on) >> >> For the DRM_DEV_ERROR() macro you can just use gm12u320->dev.dev , >> that will make the errors be printed with the in usb-interface device-name >> as prefix instead of the usb-device device-name, but that is fine. >> >> I wonder of this is all worth it then though, just to save those few bytes ? >> > > Daniel and I briefly discussed on IRC if we could make drm_device.pdev > (the PCI dev ) a legacy field in the longer term. All Linux devices > would be retrieved from drm_device.dev then. I see. Then I guess the fancy cast which I gave above is the right way to move forward with this. >> The first version made some sense since it made how drm devices with >> usb resp. pci parents are handled consistent. Now it seems to make the code >> somewhat harder to understand just to save the storage for a single >> pointer... > > What's the implication of setting drm_device.dev to the value of struct > usb_device.dev ? That's the device all the code interacts with. USB drivers bind to USB interfaces, not to the USB device (the parent of the interfaces). A single USB device may have multiple interfaces, e.g. an USB headset may implement the USB audio class for the microphone and headphones function, while having a second interface implementing the HID interface for things like volume buttons, a play/pause button, etc. So from the USB device model's pov making the USB device itself the parent of the drm device is just wrong. I guess it may not cause much issues, but it very much goes against how USB devices / interfaces are supposed to be used inside the kernel. So I guess we should just move forward with a v2 with the long / fancy casts. As for keeping my Reviewed-by for that v2, yes that is fine. Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: Store USB device in struct drm_device
Hi Hans On 22.10.20 12:17, Hans de Goede wrote: > Hi, > > On 10/22/20 11:30 AM, Thomas Zimmermann wrote: >> Hi >> >> On 22.10.20 11:20, Hans de Goede wrote: >>> Hi, >>> >>> On 10/21/20 3:07 PM, Thomas Zimmermann wrote: The drivers gm12u320 and udl operate on USB devices. They leave the PCI device in struct drm_device empty and store the USB device in their own driver structure. Fix this special case and save a few bytes by putting the USB device into an anonymous union with the PCI data. It's expected that DRM core and helpers only touch the PCI-device field for actual PCI devices. Thomas Zimmermann (3): drm: Add reference to USB device to struct drm_device drm/tiny/gm12u320: Store USB device in struct drm_device.udev drm/udl: Store USB device in struct drm_device.udev >>> >>> This series looks good to me: >>> >>> Reviewed-by: Hans de Goede >> >> Thanks a lot. Following Daniel's request, I'll drop patch 1 and instead >> do an upcast from drm_device.dev to the USB device structure. The driver >> patches 2 and 3 will be slightly different. Unless you object, I''ll >> take the r-b into the new patches. > > I somehow missed Daniel's reply about this. > > With that said, hmm that is going to be an interesting up-cast, at least > for the gm12u320, that is going to look something like this: > > struct usb_device *udev = > interface_to_usbdev(to_usb_interface(drm_dev->dev)); > > (I wrote drm_dev instead of dev to make it more clear what is going on) > > For the DRM_DEV_ERROR() macro you can just use gm12u320->dev.dev , > that will make the errors be printed with the in usb-interface device-name > as prefix instead of the usb-device device-name, but that is fine. > > I wonder of this is all worth it then though, just to save those few bytes ? > Daniel and I briefly discussed on IRC if we could make drm_device.pdev (the PCI dev ) a legacy field in the longer term. All Linux devices would be retrieved from drm_device.dev then. > The first version made some sense since it made how drm devices with > usb resp. pci parents are handled consistent. Now it seems to make the code > somewhat harder to understand just to save the storage for a single pointer... What's the implication of setting drm_device.dev to the value of struct usb_device.dev ? That's the device all the code interacts with. Best regards Thomas > > Regards, > > Hans > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: document that user-space should avoid parsing EDIDs
On Thu, Oct 22, 2020 at 10:34:44AM +, Simon Ser wrote:
> User-space should avoid parsing EDIDs for metadata already exposed via
> other KMS interfaces and properties. For instance, user-space should not
> try to extract a list of modes from the EDID: the kernel might mutate
> the mode list (because of link capabilities or quirks for instance).
>
> Other metadata not exposed by KMS can be parsed by user-space. This
> includes for instance monitor identification (make/model/serial) and
> supported color-spaces.
>
> v2: add short explanation why user-space shouldn't do this (Brian)
LGTM, thanks.
-Brian
>
> Signed-off-by: Simon Ser
> Suggested-by: Daniel Vetter
> Reviewed-by: Daniel Vetter
> Acked-by: Thomas Zimmermann
> Cc: Pekka Paalanen
> Cc: Ville Syrj�l�
> Cc: Brian Starkey
> ---
> drivers/gpu/drm/drm_connector.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 717c4e7271b0..1913d8b4e16a 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -960,6 +960,11 @@ static const struct drm_prop_enum_list dp_colorspaces[]
> = {
> * drm_connector_update_edid_property(), usually after having parsed
> * the EDID using drm_add_edid_modes(). Userspace cannot change this
> * property.
> + *
> + * User-space should not parse the EDID to obtain information exposed via
> + * other KMS properties (because the kernel might apply limits, quirks or
> + * fixups to the EDID). For instance, user-space should not try to parse
> + * mode lists from the EDID.
> * DPMS:
> * Legacy property for setting the power state of the connector. For atomic
> * drivers this is only provided for backwards compatibility with existing
> --
> 2.28.0
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm: allow drm_atomic_print_state() to accept any drm_printer
On Wed, Oct 21, 2020 at 10:01:45PM -0700, Abhinav Kumar wrote:
> Currently drm_atomic_print_state() internally allocates and uses a
> drm_info printer. Allow it to accept any drm_printer type so that
> the API can be leveraged even for taking drm snapshot.
>
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/drm_atomic.c| 17 -
> drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
> drivers/gpu/drm/drm_crtc_internal.h | 4 +++-
> 3 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 58527f151984..e7079a5f439c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1,6 +1,7 @@
> /*
> * Copyright (C) 2014 Red Hat
> * Copyright (C) 2014 Intel Corp.
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> * copy of this software and associated documentation files (the "Software"),
> @@ -1543,9 +1544,9 @@ int __drm_atomic_helper_set_config(struct drm_mode_set
> *set,
> }
> EXPORT_SYMBOL(__drm_atomic_helper_set_config);
>
> -void drm_atomic_print_state(const struct drm_atomic_state *state)
> +void drm_atomic_print_state(const struct drm_atomic_state *state,
> + struct drm_printer *p)
Please add a nice kerneldoc for this newly exported function. Specifically
this kerneldoc needs to include a warning that state updates after call
drm_atomic_state_helper_commit_hw_done() is unsafe to print using this
function, because it looks at the new state objects. Only the old state
structures will stay like this.
So maybe rename the function to say print_new_state() to make this
completely clear. That way we can eventually add a print_old_state() when
needed.
Otherwise I think this makes sense, and nicely avoids the locking issue of
looking at ->state pointers without the right locking.
-Daniel
> {
> - struct drm_printer p = drm_info_printer(state->dev->dev);
> struct drm_plane *plane;
> struct drm_plane_state *plane_state;
> struct drm_crtc *crtc;
> @@ -1554,17 +1555,23 @@ void drm_atomic_print_state(const struct
> drm_atomic_state *state)
> struct drm_connector_state *connector_state;
> int i;
>
> + if (!p) {
> + DRM_ERROR("invalid drm printer\n");
> + return;
> + }
> +
> DRM_DEBUG_ATOMIC("checking %p\n", state);
>
> for_each_new_plane_in_state(state, plane, plane_state, i)
> - drm_atomic_plane_print_state(, plane_state);
> + drm_atomic_plane_print_state(p, plane_state);
>
> for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> - drm_atomic_crtc_print_state(, crtc_state);
> + drm_atomic_crtc_print_state(p, crtc_state);
>
> for_each_new_connector_in_state(state, connector, connector_state, i)
> - drm_atomic_connector_print_state(, connector_state);
> + drm_atomic_connector_print_state(p, connector_state);
> }
> +EXPORT_SYMBOL(drm_atomic_print_state);
>
> static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p,
>bool take_locks)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 25c269bc4681..d9ae86c92608 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -2,6 +2,7 @@
> * Copyright (C) 2014 Red Hat
> * Copyright (C) 2014 Intel Corp.
> * Copyright (C) 2018 Intel Corp.
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> * copy of this software and associated documentation files (the "Software"),
> @@ -1294,6 +1295,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> struct drm_out_fence_state *fence_state;
> int ret = 0;
> unsigned int i, j, num_fences;
> + struct drm_printer p = drm_info_printer(dev->dev);
>
> /* disallow for drivers not supporting atomic: */
> if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
> @@ -1413,7 +1415,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> ret = drm_atomic_nonblocking_commit(state);
> } else {
> if (drm_debug_enabled(DRM_UT_STATE))
> - drm_atomic_print_state(state);
> + drm_atomic_print_state(state, );
>
> ret = drm_atomic_commit(state);
> }
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h
> b/drivers/gpu/drm/drm_crtc_internal.h
> index da96b2f64d7e..d34215366936 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -5,6 +5,7 @@
> * Jesse Barnes
> * Copyright © 2014 Intel Corporation
> * Daniel Vetter
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> *
> * Permission is hereby
Re: [PATCH] drm: document that user-space should avoid parsing EDIDs
On Friday, October 9, 2020 12:25 PM, Brian Starkey wrote: > I assume that this is so that the kernel can apply quirks/limits in > cases where it knows it needs to? I think it would be nice to put at > least a few words indicating "why", otherwise this feels like an > arbitrary commandment with no justification. I mainly wanted to avoid having some user-space developers think "but I know better than the kernel!". I don't want to document that the kernel will exclusively change the mode list because of quirks, then figuring out that we need to fix-up another EDID field for other reasons, and end up with complains. I added an intentionally short explanation in v2. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: document that user-space should avoid parsing EDIDs
User-space should avoid parsing EDIDs for metadata already exposed via
other KMS interfaces and properties. For instance, user-space should not
try to extract a list of modes from the EDID: the kernel might mutate
the mode list (because of link capabilities or quirks for instance).
Other metadata not exposed by KMS can be parsed by user-space. This
includes for instance monitor identification (make/model/serial) and
supported color-spaces.
v2: add short explanation why user-space shouldn't do this (Brian)
Signed-off-by: Simon Ser
Suggested-by: Daniel Vetter
Reviewed-by: Daniel Vetter
Acked-by: Thomas Zimmermann
Cc: Pekka Paalanen
Cc: Ville Syrjälä
Cc: Brian Starkey
---
drivers/gpu/drm/drm_connector.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 717c4e7271b0..1913d8b4e16a 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -960,6 +960,11 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
* drm_connector_update_edid_property(), usually after having parsed
* the EDID using drm_add_edid_modes(). Userspace cannot change this
* property.
+ *
+ * User-space should not parse the EDID to obtain information exposed via
+ * other KMS properties (because the kernel might apply limits, quirks or
+ * fixups to the EDID). For instance, user-space should not try to parse
+ * mode lists from the EDID.
* DPMS:
* Legacy property for setting the power state of the connector. For atomic
* drivers this is only provided for backwards compatibility with existing
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/10] drm/gem: Store client buffer mappings as struct dma_buf_map
On Thu, Oct 22, 2020 at 11:18 AM Thomas Zimmermann wrote:
>
> Hi
>
> On 22.10.20 10:49, Daniel Vetter wrote:
> > On Tue, Oct 20, 2020 at 02:20:44PM +0200, Thomas Zimmermann wrote:
> >> Kernel DRM clients now store their framebuffer address in an instance
> >> of struct dma_buf_map. Depending on the buffer's location, the address
> >> refers to system or I/O memory.
> >>
> >> Callers of drm_client_buffer_vmap() receive a copy of the value in
> >> the call's supplied arguments. It can be accessed and modified with
> >> dma_buf_map interfaces.
> >>
> >> Signed-off-by: Thomas Zimmermann
> >> Reviewed-by: Daniel Vetter
> >> Tested-by: Sam Ravnborg
> >> ---
> >> drivers/gpu/drm/drm_client.c| 34 +++--
> >> drivers/gpu/drm/drm_fb_helper.c | 23 +-
> >> include/drm/drm_client.h| 7 ---
> >> 3 files changed, 38 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> >> index ac0082bed966..fe573acf1067 100644
> >> --- a/drivers/gpu/drm/drm_client.c
> >> +++ b/drivers/gpu/drm/drm_client.c
> >> @@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
> >> drm_client_buffer *buffer)
> >> {
> >> struct drm_device *dev = buffer->client->dev;
> >>
> >> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
> >> +drm_gem_vunmap(buffer->gem, >map);
> >>
> >> if (buffer->gem)
> >> drm_gem_object_put(buffer->gem);
> >> @@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev
> >> *client, u32 width, u32 height, u
> >> /**
> >> * drm_client_buffer_vmap - Map DRM client buffer into address space
> >> * @buffer: DRM client buffer
> >> + * @map_copy: Returns the mapped memory's address
> >> *
> >> * This function maps a client buffer into kernel address space. If the
> >> - * buffer is already mapped, it returns the mapping's address.
> >> + * buffer is already mapped, it returns the existing mapping's address.
> >> *
> >> * Client buffer mappings are not ref'counted. Each call to
> >> * drm_client_buffer_vmap() should be followed by a call to
> >> * drm_client_buffer_vunmap(); or the client buffer should be mapped
> >> * throughout its lifetime.
> >> *
> >> + * The returned address is a copy of the internal value. In contrast to
> >> + * other vmap interfaces, you don't need it for the client's vunmap
> >> + * function. So you can modify it at will during blit and draw operations.
> >> + *
> >> * Returns:
> >> - * The mapped memory's address
> >> + * 0 on success, or a negative errno code otherwise.
> >> */
> >> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> >> +int
> >> +drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct
> >> dma_buf_map *map_copy)
> >> {
> >> -struct dma_buf_map map;
> >> +struct dma_buf_map *map = >map;
> >> int ret;
> >>
> >> -if (buffer->vaddr)
> >> -return buffer->vaddr;
> >> +if (dma_buf_map_is_set(map))
> >> +goto out;
> >>
> >> /*
> >> * FIXME: The dependency on GEM here isn't required, we could
> >> @@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct
> >> drm_client_buffer *buffer)
> >> * fd_install step out of the driver backend hooks, to make that
> >> * final step optional for internal users.
> >> */
> >> -ret = drm_gem_vmap(buffer->gem, );
> >> +ret = drm_gem_vmap(buffer->gem, map);
> >> if (ret)
> >> -return ERR_PTR(ret);
> >> +return ret;
> >>
> >> -buffer->vaddr = map.vaddr;
> >> +out:
> >> +*map_copy = *map;
> >>
> >> -return map.vaddr;
> >> +return 0;
> >> }
> >> EXPORT_SYMBOL(drm_client_buffer_vmap);
> >>
> >> @@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
> >> */
> >> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> >> {
> >> -struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
> >> +struct dma_buf_map *map = >map;
> >>
> >> -drm_gem_vunmap(buffer->gem, );
> >> -buffer->vaddr = NULL;
> >> +drm_gem_vunmap(buffer->gem, map);
> >> }
> >> EXPORT_SYMBOL(drm_client_buffer_vunmap);
> >>
> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> >> b/drivers/gpu/drm/drm_fb_helper.c
> >> index c2f72bb6afb1..6212cd7cde1d 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -378,7 +378,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> >> drm_fb_helper *fb_helper,
> >> unsigned int cpp = fb->format->cpp[0];
> >> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> >> void *src = fb_helper->fbdev->screen_buffer + offset;
> >> -void *dst = fb_helper->buffer->vaddr + offset;
> >> +void *dst = fb_helper->buffer->map.vaddr + offset;
> >> size_t len = (clip->x2 - clip->x1) * cpp;
> >> unsigned int y;
> >>
> >> @@ -400,7 +400,8 @@ static void drm_fb_helper_dirty_work(struct
> >>
Re: [PATCH 0/3] drm: Store USB device in struct drm_device
Hi, On 10/22/20 11:30 AM, Thomas Zimmermann wrote: > Hi > > On 22.10.20 11:20, Hans de Goede wrote: >> Hi, >> >> On 10/21/20 3:07 PM, Thomas Zimmermann wrote: >>> The drivers gm12u320 and udl operate on USB devices. They leave the >>> PCI device in struct drm_device empty and store the USB device in their >>> own driver structure. >>> >>> Fix this special case and save a few bytes by putting the USB device >>> into an anonymous union with the PCI data. It's expected that DRM >>> core and helpers only touch the PCI-device field for actual PCI devices. >>> >>> Thomas Zimmermann (3): >>> drm: Add reference to USB device to struct drm_device >>> drm/tiny/gm12u320: Store USB device in struct drm_device.udev >>> drm/udl: Store USB device in struct drm_device.udev >> >> This series looks good to me: >> >> Reviewed-by: Hans de Goede > > Thanks a lot. Following Daniel's request, I'll drop patch 1 and instead > do an upcast from drm_device.dev to the USB device structure. The driver > patches 2 and 3 will be slightly different. Unless you object, I''ll > take the r-b into the new patches. I somehow missed Daniel's reply about this. With that said, hmm that is going to be an interesting up-cast, at least for the gm12u320, that is going to look something like this: struct usb_device *udev = interface_to_usbdev(to_usb_interface(drm_dev->dev)); (I wrote drm_dev instead of dev to make it more clear what is going on) For the DRM_DEV_ERROR() macro you can just use gm12u320->dev.dev , that will make the errors be printed with the in usb-interface device-name as prefix instead of the usb-device device-name, but that is fine. I wonder of this is all worth it then though, just to save those few bytes ? The first version made some sense since it made how drm devices with usb resp. pci parents are handled consistent. Now it seems to make the code somewhat harder to understand just to save the storage for a single pointer... Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] drm_modes: signed integer overflow
On Wed, Oct 21, 2020 at 08:13:43PM -0700, Randy Dunlap wrote: > Hi, > > With linux-next 20201021, when booting up, I am seeing this: > > [0.560896] UBSAN: signed-integer-overflow in > ../drivers/gpu/drm/drm_modes.c:765:20 > [0.560903] 2376000 * 1000 cannot be represented in type 'int' Dang. Didn't realize these new crazy >8k modes have dotclocks reaching almost 6 GHz, which would overflow even u32. I guess we'll switch to 64bit maths. Now I wonder how many other places can hit this overflow in practice... > [0.560909] CPU: 3 PID: 7 Comm: kworker/u16:0 Not tainted > 5.9.0-next-20201021 #2 > [0.560914] Hardware name: TOSHIBA PORTEGE R835/Portable PC, BIOS Version > 4.10 01/08/2013 > [0.560924] Workqueue: events_unbound async_run_entry_fn > > [0.560930] Call Trace: > [0.560938] dump_stack+0x5e/0x74 > [0.560943] ubsan_epilogue+0x9/0x45 > [0.560948] handle_overflow+0x8b/0x98 > [0.560953] ? set_track+0x3f/0xad > [0.560958] __ubsan_handle_mul_overflow+0xe/0x10 > [0.560964] drm_mode_vrefresh+0x4a/0xbc > [0.560970] initcall i915_init+0x0/0x6a returned 0 after 116076 usecs > [0.560974] calling cn_proc_init+0x0/0x36 @ 1 > [0.560978] cea_mode_alternate_clock+0x11/0x62 > [0.560985] drm_match_cea_mode+0xc7/0x1e7 > [0.560987] initcall cn_proc_init+0x0/0x36 returned 0 after 3 usecs > [0.560990] calling topology_sysfs_init+0x0/0x2d @ 1 > [0.561000] drm_mode_validate_ycbcr420+0xd/0x48 > [0.561005] drm_helper_probe_single_connector_modes+0x6db/0x7da > [0.561012] drm_client_modeset_probe+0x225/0x143f > [0.561018] ? bitmap_fold+0x8a/0x8a > [0.561023] ? update_cfs_rq_load_avg+0x192/0x1a2 > [0.561029] __drm_fb_helper_initial_config_and_unlock+0x3f/0x5b7 > [0.561035] ? get_sd_balance_interval+0x1c/0x40 > [0.561040] drm_fb_helper_initial_config+0x48/0x4f > [0.561047] intel_fbdev_initial_config+0x13/0x23 > [0.561052] async_run_entry_fn+0x89/0x15c > [0.561058] process_one_work+0x15c/0x1f3 > [0.561064] worker_thread+0x1ac/0x25d > [0.561069] ? process_scheduled_works+0x2e/0x2e > [0.561074] kthread+0x10e/0x116 > [0.561078] ? kthread_parkme+0x1c/0x1c > [0.561083] ret_from_fork+0x22/0x30 > [0.561087] > > > -- > ~Randy > Reported-by: Randy Dunlap > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm: allow drm_atomic_print_state() to accept any drm_printer
Hi Abhinav, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-exynos/exynos-drm-next] [also build test WARNING on drm-intel/for-linux-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.9 next-20201022] [cannot apply to drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507 base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next config: microblaze-randconfig-r003-20201022 (attached as .config) compiler: microblaze-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/76add8f28420cad0b0d2977abed6e031ef307f32 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507 git checkout 76add8f28420cad0b0d2977abed6e031ef307f32 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=microblaze If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/selftests/test-drm_framebuffer.c:12: >> drivers/gpu/drm/selftests/../drm_crtc_internal.h:238:10: warning: 'struct >> drm_printer' declared inside parameter list will not be visible outside of >> this definition or declaration 238 | struct drm_printer *p); | ^~~ drivers/gpu/drm/selftests/test-drm_framebuffer.c: In function 'execute_drm_mode_fb_cmd2': drivers/gpu/drm/selftests/test-drm_framebuffer.c:333:26: warning: variable 'fb' set but not used [-Wunused-but-set-variable] 333 | struct drm_framebuffer *fb; | ^~ vim +238 drivers/gpu/drm/selftests/../drm_crtc_internal.h 231 232 int __drm_atomic_helper_disable_plane(struct drm_plane *plane, 233struct drm_plane_state *plane_state); 234 int __drm_atomic_helper_set_config(struct drm_mode_set *set, 235 struct drm_atomic_state *state); 236 237 void drm_atomic_print_state(const struct drm_atomic_state *state, > 238 struct drm_printer *p); 239 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: document that blobs are ref'counted
Thanks for this!
I can't review the correctness, but the description looks clear to me
so,
Reviewed-by: Jonas Ådahl
Jonas
On Thu, Oct 22, 2020 at 09:38:05AM +, Simon Ser wrote:
> User-space doesn't need to keep track of blobs that might be in use by
> the kernel. User-space can just destroy blobs as soon as they don't need
> them anymore.
>
> Signed-off-by: Simon Ser
> Cc: Pekka Paalanen
> Cc: Daniel Vetter
> Cc: Jonas Ådahl
> ---
> include/uapi/drm/drm_mode.h | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 863eda048265..f7c41aa4b5eb 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -924,6 +924,10 @@ struct drm_mode_create_blob {
> * struct drm_mode_destroy_blob - Destroy user blob
> * @blob_id: blob_id to destroy
> * Destroy a user-created blob property.
> + *
> + * Blobs are reference-counted by the kernel, so user-space can destroy them
> as
> + * soon as they're done with them. For instance user-space can destroy a
> blob
> + * used in an atomic commit right after performing the atomic commit ioctl.
> */
> struct drm_mode_destroy_blob {
> __u32 blob_id;
> --
> 2.28.0
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: document that blobs are ref'counted
User-space doesn't need to keep track of blobs that might be in use by
the kernel. User-space can just destroy blobs as soon as they don't need
them anymore.
Signed-off-by: Simon Ser
Cc: Pekka Paalanen
Cc: Daniel Vetter
Cc: Jonas Ådahl
---
include/uapi/drm/drm_mode.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 863eda048265..f7c41aa4b5eb 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -924,6 +924,10 @@ struct drm_mode_create_blob {
* struct drm_mode_destroy_blob - Destroy user blob
* @blob_id: blob_id to destroy
* Destroy a user-created blob property.
+ *
+ * Blobs are reference-counted by the kernel, so user-space can destroy them as
+ * soon as they're done with them. For instance user-space can destroy a blob
+ * used in an atomic commit right after performing the atomic commit ioctl.
*/
struct drm_mode_destroy_blob {
__u32 blob_id;
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 12/16] drm/i915/hdcp: MST streams support in hdcp port_data
Add support for multiple mst stream in hdcp port data
which will be used by RepeaterAuthStreamManage msg and
HDCP 2.2 security f/w for m' validation.
v2:
Init the hdcp port data k for HDMI/DP SST strem.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
.../drm/i915/display/intel_display_types.h| 4 +-
drivers/gpu/drm/i915/display/intel_hdcp.c | 100 +++---
2 files changed, 90 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 749c3a7e0b45..24e0067c2e7c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1445,10 +1445,12 @@ struct intel_digital_port {
enum phy_fia tc_phy_fia;
u8 tc_phy_fia_idx;
- /* protects num_hdcp_streams reference count, port_data */
+ /* protects num_hdcp_streams reference count, port_data and port_auth */
struct mutex hdcp_mutex;
/* the number of pipes using HDCP signalling out of this port */
unsigned int num_hdcp_streams;
+ /* port HDCP auth status */
+ bool port_auth;
/* HDCP port data need to pass to security f/w */
struct hdcp_port_data port_data;
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 207fa17129ae..2e719df1e5b1 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -26,6 +26,62 @@
#define KEY_LOAD_TRIES 5
#define HDCP2_LC_RETRY_CNT 3
+static int intel_conn_to_vcpi(struct intel_connector *connector)
+{
+ /* For HDMI this is forced to be 0x0. For DP SST also this is 0x0. */
+ return connector->port ? connector->port->vcpi.vcpi : 0;
+}
+
+static int
+intel_hdcp_required_content_stream(struct intel_digital_port *dig_port)
+{
+ struct drm_connector_list_iter conn_iter;
+ struct intel_digital_port *conn_dig_port;
+ struct intel_connector *connector;
+ struct intel_hdcp *hdcp;
+ struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+ struct hdcp_port_data *data = _port->port_data;
+ bool enforce_type0 = false;
+ int k;
+
+ if (dig_port->port_auth)
+ return 0;
+
+ drm_connector_list_iter_begin(>drm, _iter);
+ for_each_intel_connector_iter(connector, _iter) {
+ if (!intel_encoder_is_mst(intel_attached_encoder(connector)))
+ continue;
+
+ conn_dig_port = intel_attached_dig_port(connector);
+ if (conn_dig_port != dig_port)
+ continue;
+
+ if (connector->base.status == connector_status_disconnected)
+ continue;
+
+ hdcp = >hdcp;
+ if (!enforce_type0 && (hdcp->content_type &&
!intel_hdcp2_capable(connector)))
+ enforce_type0 = true;
+
+ data->streams[data->k].stream_id =
intel_conn_to_vcpi(connector);
+ data->k++;
+
+ /* if there is only one active stream */
+ if (dig_port->dp.active_mst_links <= 1)
+ break;
+ }
+ drm_connector_list_iter_end(_iter);
+
+ if (drm_WARN_ON(>drm, data->k > INTEL_NUM_PIPES(i915) || data->k
== 0))
+ return -EINVAL;
+
+ for (k = 0; k < data->k; k++)
+ data->streams[k].stream_type =
+ enforce_type0 ? DRM_MODE_HDCP_CONTENT_TYPE0 :
hdcp->content_type;
+
+ return 0;
+}
+
static
bool intel_hdcp_is_ksv_valid(u8 *ksv)
{
@@ -1296,6 +1352,7 @@ static int hdcp2_authenticate_port(struct intel_connector
*connector)
if (ret < 0)
drm_dbg_kms(_priv->drm, "Enable hdcp auth failed. %d\n",
ret);
+
mutex_unlock(_priv->hdcp_comp_mutex);
return ret;
@@ -1477,13 +1534,14 @@ static
int _hdcp2_propagate_stream_management_info(struct intel_connector *connector)
{
struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
+ struct hdcp_port_data *data = _port->port_data;
struct intel_hdcp *hdcp = >hdcp;
union {
struct hdcp2_rep_stream_manage stream_manage;
struct hdcp2_rep_stream_ready stream_ready;
} msgs;
const struct intel_hdcp_shim *shim = hdcp->shim;
- int ret;
+ int ret, streams_size_delta, i;
if (connector->hdcp.seq_num_m > HDCP_2_2_SEQ_NUM_MAX)
return -ERANGE;
@@ -1493,15 +1551,18 @@ int _hdcp2_propagate_stream_management_info(struct
intel_connector *connector)
drm_hdcp_cpu_to_be24(msgs.stream_manage.seq_num_m, hdcp->seq_num_m);
/* K no of streams is fixed as 1. Stored as big-endian. */
- msgs.stream_manage.k = cpu_to_be16(1);
+ msgs.stream_manage.k = cpu_to_be16(data->k);
- /* For HDMI
[PATCH v3 14/16] drm/i915/hdcp: Add HDCP 2.2 stream register
Add HDCP 2.2 DP MST HDCP2_STREAM_STATUS
and HDCP2_AUTH_STREAM register in i915_reg header.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/i915_reg.h | 30 ++
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 86a9a5145e47..cb6ec2c241f2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9882,6 +9882,7 @@ enum skl_power_gate {
_PORTD_HDCP2_BASE, \
_PORTE_HDCP2_BASE, \
_PORTF_HDCP2_BASE) + (x))
+
#define PORT_HDCP2_AUTH(port) _PORT_HDCP2_BASE(port, 0x98)
#define _TRANSA_HDCP2_AUTH 0x66498
#define _TRANSB_HDCP2_AUTH 0x66598
@@ -9921,6 +9922,35 @@ enum skl_power_gate {
TRANS_HDCP2_STATUS(trans) : \
PORT_HDCP2_STATUS(port))
+#define PORT_HDCP2_STREAM_STATUS(port) _PORT_HDCP2_BASE(port, 0xC0)
+#define _TRANSA_HDCP2_STREAM_STATUS0x664C0
+#define _TRANSB_HDCP2_STREAM_STATUS0x665C0
+#define TRANS_HDCP2_STREAM_STATUS(trans) _MMIO_TRANS(trans, \
+
_TRANSA_HDCP2_STREAM_STATUS, \
+ _TRANSB_HDCP2_STREAM_STATUS)
+#define STREAM_ENCRYPTION_STATUS BIT(31)
+#define STREAM_TYPE_STATUS BIT(30)
+#define HDCP2_STREAM_STATUS(dev_priv, trans, port) \
+ (INTEL_GEN(dev_priv) >= 12 ? \
+TRANS_HDCP2_STREAM_STATUS(trans) : \
+PORT_HDCP2_STREAM_STATUS(port))
+
+#define _PORTA_HDCP2_AUTH_STREAM 0x66F00
+#define _PORTB_HDCP2_AUTH_STREAM 0x66F04
+#define PORT_HDCP2_AUTH_STREAM(port) _MMIO_PORT(port, \
+ _PORTA_HDCP2_AUTH_STREAM, \
+ _PORTB_HDCP2_AUTH_STREAM)
+#define _TRANSA_HDCP2_AUTH_STREAM 0x66F00
+#define _TRANSB_HDCP2_AUTH_STREAM 0x66F04
+#define TRANS_HDCP2_AUTH_STREAM(trans) _MMIO_TRANS(trans, \
+ _TRANSA_HDCP2_AUTH_STREAM, \
+ _TRANSB_HDCP2_AUTH_STREAM)
+#define AUTH_STREAM_TYPE BIT(31)
+#define HDCP2_AUTH_STREAM(dev_priv, trans, port) \
+ (INTEL_GEN(dev_priv) >= 12 ? \
+TRANS_HDCP2_AUTH_STREAM(trans) : \
+PORT_HDCP2_AUTH_STREAM(port))
+
/* Per-pipe DDI Function Control */
#define _TRANS_DDI_FUNC_CTL_A 0x60400
#define _TRANS_DDI_FUNC_CTL_B 0x61400
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 13/16] drm/i915/hdcp: Pass connector to check_2_2_link
This requires for HDCP 2.2 MST check link.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_display_types.h | 3 ++-
drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 3 ++-
drivers/gpu/drm/i915/display/intel_hdcp.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++-
4 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 24e0067c2e7c..dfb5be64e03a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -375,7 +375,8 @@ struct intel_hdcp_shim {
bool is_repeater, u8 type);
/* HDCP2.2 Link Integrity Check */
- int (*check_2_2_link)(struct intel_digital_port *dig_port);
+ int (*check_2_2_link)(struct intel_digital_port *dig_port,
+ struct intel_connector *connector);
};
struct intel_hdcp {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
index 384e384cb9e2..a0c62e363c39 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
@@ -585,7 +585,8 @@ int intel_dp_hdcp2_config_stream_type(struct
intel_digital_port *dig_port,
}
static
-int intel_dp_hdcp2_check_link(struct intel_digital_port *dig_port)
+int intel_dp_hdcp2_check_link(struct intel_digital_port *dig_port,
+ struct intel_connector *connector)
{
u8 rx_status;
int ret;
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 2e719df1e5b1..86a3ffadd97f 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -1942,7 +1942,7 @@ static int intel_hdcp2_check_link(struct intel_connector
*connector)
goto out;
}
- ret = hdcp->shim->check_2_2_link(dig_port);
+ ret = hdcp->shim->check_2_2_link(dig_port, connector);
if (ret == HDCP_LINK_PROTECTED) {
if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
intel_hdcp_update_value(connector,
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 0788de04711b..bd0d91101464 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1734,7 +1734,8 @@ int intel_hdmi_hdcp2_read_msg(struct intel_digital_port
*dig_port,
}
static
-int intel_hdmi_hdcp2_check_link(struct intel_digital_port *dig_port)
+int intel_hdmi_hdcp2_check_link(struct intel_digital_port *dig_port,
+ struct intel_connector *connector)
{
u8 rx_status[HDCP_2_2_HDMI_RXSTATUS_LEN];
int ret;
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 16/16] drm/i915/hdcp: Enable HDCP 2.2 MST support
Enable HDCP 2.2 over DP MST.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_hdcp.c | 46 ++-
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 86a3ffadd97f..98225777c9f9 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -1696,6 +1696,32 @@ static int hdcp2_authenticate_sink(struct
intel_connector *connector)
return ret;
}
+static int hdcp2_enable_stream_encryption(struct intel_connector *connector)
+{
+ struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ struct intel_hdcp *hdcp = >hdcp;
+ enum transcoder cpu_transcoder = hdcp->cpu_transcoder;
+ enum port port = dig_port->base.port;
+ int ret = 0;
+
+ if (!(intel_de_read(dev_priv, HDCP2_STATUS(dev_priv, cpu_transcoder,
port)) &
+ LINK_ENCRYPTION_STATUS)) {
+ drm_err(_priv->drm, "HDCP 2.2 Link is not encrypted\n");
+ return -EPERM;
+ }
+
+ if (hdcp->shim->stream_2_2_encryption) {
+ ret = hdcp->shim->stream_2_2_encryption(dig_port, true);
+ if (ret) {
+ drm_err(_priv->drm, "Failed to enable HDCP 2.2
stream enc\n");
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
static int hdcp2_enable_encryption(struct intel_connector *connector)
{
struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
@@ -1834,7 +1860,7 @@ static int hdcp2_authenticate_and_encrypt(struct
intel_connector *connector)
drm_dbg_kms(>drm, "Port deauth failed.\n");
}
- if (!ret) {
+ if (!ret && !dig_port->port_auth) {
/*
* Ensuring the required 200mSec min time interval between
* Session Key Exchange and encryption.
@@ -1849,6 +1875,8 @@ static int hdcp2_authenticate_and_encrypt(struct
intel_connector *connector)
}
}
+ ret = hdcp2_enable_stream_encryption(connector);
+
return ret;
}
@@ -1893,11 +1921,23 @@ static int _intel_hdcp2_disable(struct intel_connector
*connector)
struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
struct drm_i915_private *i915 = to_i915(connector->base.dev);
struct hdcp_port_data *data = _port->port_data;
+ struct intel_hdcp *hdcp = >hdcp;
int ret;
drm_dbg_kms(>drm, "[%s:%d] HDCP2.2 is being Disabled\n",
connector->base.name, connector->base.base.id);
+ if (hdcp->shim->stream_2_2_encryption) {
+ ret = hdcp->shim->stream_2_2_encryption(dig_port, false);
+ if (ret) {
+ drm_err(>drm, "Failed to disable HDCP 2.2 stream
enc\n");
+ return ret;
+ }
+ }
+
+ if (dig_port->num_hdcp_streams > 0)
+ return ret;
+
ret = hdcp2_disable_encryption(connector);
if (hdcp2_deauthenticate_port(connector) < 0)
@@ -1921,6 +1961,7 @@ static int intel_hdcp2_check_link(struct intel_connector
*connector)
int ret = 0;
mutex_lock(>mutex);
+ mutex_lock(_port->hdcp_mutex);
cpu_transcoder = hdcp->cpu_transcoder;
/* hdcp2_check_link is expected only when HDCP2.2 is Enabled */
@@ -1998,6 +2039,7 @@ static int intel_hdcp2_check_link(struct intel_connector
*connector)
}
out:
+ mutex_unlock(_port->hdcp_mutex);
mutex_unlock(>mutex);
return ret;
}
@@ -2179,7 +2221,7 @@ int intel_hdcp_init(struct intel_connector *connector,
if (!shim)
return -EINVAL;
- if (is_hdcp2_supported(dev_priv) && !connector->mst_port)
+ if (is_hdcp2_supported(dev_priv))
intel_hdcp2_init(connector, dig_port, shim);
ret =
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 15/16] drm/i915/hdcp: Support for HDCP 2.2 MST shim callbacks
Add support for HDCP 2.2 DP MST shim callback.
This adds existing DP HDCP shim callback for Link Authentication
and Encryption and HDCP 2.2 stream encryption
callback.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
.../drm/i915/display/intel_display_types.h| 4 +
drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 81 +--
2 files changed, 77 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index dfb5be64e03a..4cbb151ff3cf 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -374,6 +374,10 @@ struct intel_hdcp_shim {
int (*config_stream_type)(struct intel_digital_port *dig_port,
bool is_repeater, u8 type);
+ /* Enable/Disable HDCP 2.2 stream encryption on DP MST Transport Link */
+ int (*stream_2_2_encryption)(struct intel_digital_port *dig_port,
+bool enable);
+
/* HDCP2.2 Link Integrity Check */
int (*check_2_2_link)(struct intel_digital_port *dig_port,
struct intel_connector *connector);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
index a0c62e363c39..7e45b9964a29 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
@@ -698,18 +698,14 @@ intel_dp_mst_hdcp_strem_encryption(struct
intel_digital_port *dig_port,
return 0;
}
-static
-bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port,
- struct intel_connector *connector)
+static bool intel_dp_mst_get_qses_status(struct intel_digital_port *dig_port,
+struct intel_connector *connector)
{
struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
- struct intel_dp *intel_dp = _port->dp;
struct drm_dp_query_stream_enc_status_ack_reply reply;
+ struct intel_dp *intel_dp = _port->dp;
int ret;
- if (!intel_dp_hdcp_check_link(dig_port, connector))
- return false;
-
ret = drm_dp_send_query_stream_enc_status(_dp->mst_mgr,
connector->port, );
if (ret) {
@@ -722,6 +718,70 @@ bool intel_dp_mst_hdcp_check_link(struct
intel_digital_port *dig_port,
return reply.auth_completed && reply.encryption_enabled;
}
+static
+bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port,
+ struct intel_connector *connector)
+{
+ if (!intel_dp_hdcp_check_link(dig_port, connector))
+ return false;
+
+ return intel_dp_mst_get_qses_status(dig_port, connector);
+}
+
+static int
+intel_dp_mst_hdcp2_strem_encryption(struct intel_digital_port *dig_port,
+ bool enable)
+{
+ struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+ struct intel_dp *dp = _port->dp;
+ struct intel_hdcp *hdcp = >attached_connector->hdcp;
+ enum port port = dig_port->base.port;
+ /* HDCP2.x register uses stream transcoder */
+ enum transcoder cpu_transcoder = hdcp->stream_transcoder;
+ int ret;
+
+ if (enable && (intel_de_read(i915, HDCP2_AUTH_STREAM(i915,
cpu_transcoder, port)) &
+ AUTH_STREAM_TYPE) != hdcp->content_type) {
+ drm_err(>drm, "Seurity f/w didn't set correct auth
strem_type\n");
+ return -EINVAL;
+ }
+
+ ret = intel_dp_mst_toggle_select_hdcp_stream(dig_port, enable);
+ if (ret)
+ return ret;
+
+ /* Wait for encryption confirmation */
+ if (intel_de_wait_for_register(i915,
+ HDCP2_STREAM_STATUS(i915,
cpu_transcoder, port),
+ STREAM_ENCRYPTION_STATUS,
+ enable ? STREAM_ENCRYPTION_STATUS : 0,
+ HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
+ drm_err(>drm, "Timed out waiting for stream encryption
%s\n",
+ enable ? "enabled" : "disabled");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+/*
+ * DP v2.0 I.3.3 ignore the stream signature L' is QSES reply msg reply.
+ * I.3.5 MST source device may use a QSES msg to query downstream status
+ * for a particular stream.
+ */
+static
+int intel_dp_mst_hdcp2_check_link(struct intel_digital_port *dig_port,
+ struct intel_connector *connector)
+{
+ int ret;
+
+ ret = intel_dp_hdcp2_check_link(dig_port, connector);
+ if (ret)
+ return ret;
+
+ return intel_dp_mst_get_qses_status(dig_port, connector) ? 0 : -EINVAL;
+}
+
static const struct intel_hdcp_shim
[PATCH v3 00/16] HDCP 2.2 DP MST Support
v3 version of this series has fixed the CI reported failures and added below patch in this series. [PATCH v3 02/16] drm/i915/hdcp: Get conn while content_type changed It has also added the Ack of Tomas to merge the mei_hdcp.c patch via drm-intel. This series has been tested with IGT changes to do a single commit to enable HDCP on all DP-MST connector. HDCP 2.2 support over DP MST actually starts from below patch [PATCH v3 08/16] drm/i915/hdcp: Pass dig_port to intel_hdcp_init. Gen12 HDCP 1.4 support of this series has also floated separately with below series. (https://patchwork.freedesktop.org/series/82605/) Anshuman Gupta (16): drm/i915/hdcp: Update CP property in update_pipe drm/i915/hdcp: Get conn while content_type changed drm/i915/hotplug: Handle CP_IRQ for DP-MST drm/i915/hdcp: DP MST transcoder for link and stream drm/i915/hdcp: Move HDCP enc status timeout to header drm/i915/hdcp: HDCP stream encryption support drm/i915/hdcp: Enable Gen12 HDCP 1.4 DP MST support drm/i915/hdcp: Pass dig_port to intel_hdcp_init drm/i915/hdcp: Encapsulate hdcp_port_data to dig_port misc/mei/hdcp: Fix AUTH_STREAM_REQ cmd buffer len drm/hdcp: Max MST content streams drm/i915/hdcp: MST streams support in hdcp port_data drm/i915/hdcp: Pass connector to check_2_2_link drm/i915/hdcp: Add HDCP 2.2 stream register drm/i915/hdcp: Support for HDCP 2.2 MST shim callbacks drm/i915/hdcp: Enable HDCP 2.2 MST support drivers/gpu/drm/i915/display/intel_ddi.c | 14 +- drivers/gpu/drm/i915/display/intel_ddi.h | 6 +- .../drm/i915/display/intel_display_types.h| 20 +- drivers/gpu/drm/i915/display/intel_dp.c | 14 +- drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 168 +-- drivers/gpu/drm/i915/display/intel_dp_mst.c | 12 +- drivers/gpu/drm/i915/display/intel_hdcp.c | 282 ++ drivers/gpu/drm/i915/display/intel_hdcp.h | 8 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 19 +- drivers/gpu/drm/i915/i915_reg.h | 31 ++ drivers/misc/mei/hdcp/mei_hdcp.c | 3 +- include/drm/drm_hdcp.h| 8 +- 12 files changed, 465 insertions(+), 120 deletions(-) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 06/16] drm/i915/hdcp: HDCP stream encryption support
Both HDCP_{1.x,2.x} requires to select/deselect Multistream HDCP bit
in TRANS_DDI_FUNC_CTL in order to enable/disable stream HDCP
encryption over DP MST Transport Link.
HDCP 1.4 stream encryption requires to validate the stream encryption
status in HDCP_STATUS_{TRANSCODER,PORT} register driving that link
in order to enable/disable the stream encryption.
Both of above requirement are same for all Gen with respect to
B.Spec Documentation.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_ddi.c | 10 +--
drivers/gpu/drm/i915/display/intel_ddi.h | 6 +-
.../drm/i915/display/intel_display_types.h| 4 +
drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 80 ---
drivers/gpu/drm/i915/display/intel_hdmi.c | 14 ++--
drivers/gpu/drm/i915/i915_reg.h | 1 +
6 files changed, 90 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index bf8730267cfd..fbeffdfd1a0d 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1948,9 +1948,9 @@ void intel_ddi_disable_transcoder_func(const struct
intel_crtc_state *crtc_state
}
}
-int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
-enum transcoder cpu_transcoder,
-bool enable)
+int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
+ enum transcoder cpu_transcoder,
+ bool enable, u32 hdcp_mask)
{
struct drm_device *dev = intel_encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -1965,9 +1965,9 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder
*intel_encoder,
tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder));
if (enable)
- tmp |= TRANS_DDI_HDCP_SIGNALLING;
+ tmp |= hdcp_mask;
else
- tmp &= ~TRANS_DDI_HDCP_SIGNALLING;
+ tmp &= ~hdcp_mask;
intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder), tmp);
intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h
b/drivers/gpu/drm/i915/display/intel_ddi.h
index dcc711cfe4fe..a4dd815c 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.h
+++ b/drivers/gpu/drm/i915/display/intel_ddi.h
@@ -50,9 +50,9 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state);
u32 ddi_signal_levels(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state);
-int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
-enum transcoder cpu_transcoder,
-bool enable);
+int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
+ enum transcoder cpu_transcoder,
+ bool enable, u32 hdcp_mask);
void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
#endif /* __INTEL_DDI_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index c47124a679b6..59b8fc21e3e8 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -339,6 +339,10 @@ struct intel_hdcp_shim {
enum transcoder cpu_transcoder,
bool enable);
+ /* Enable/Disable stream encryption on DP MST Transport Link */
+ int (*stream_encryption)(struct intel_digital_port *dig_port,
+bool enable);
+
/* Ensures the link is still protected */
bool (*check_link)(struct intel_digital_port *dig_port,
struct intel_connector *connector);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
index 03424d20e9f7..652d4645f255 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
@@ -16,6 +16,30 @@
#include "intel_dp.h"
#include "intel_hdcp.h"
+static unsigned int transcoder_to_stream_enc_status(enum transcoder
cpu_transcoder)
+{
+ u32 stream_enc_mask;
+
+ switch (cpu_transcoder) {
+ case TRANSCODER_A:
+ stream_enc_mask = HDCP_STATUS_STREAM_A_ENC;
+ break;
+ case TRANSCODER_B:
+ stream_enc_mask = HDCP_STATUS_STREAM_B_ENC;
+ break;
+ case TRANSCODER_C:
+ stream_enc_mask = HDCP_STATUS_STREAM_C_ENC;
+ break;
+ case TRANSCODER_D:
+ stream_enc_mask = HDCP_STATUS_STREAM_D_ENC;
+ break;
+
[PATCH v3 11/16] drm/hdcp: Max MST content streams
Let's define Maximum MST content streams up to four generically which can be supported by modern display controllers. Cc: Sean Paul Cc: Ramalingam C Acked-by: Maarten Lankhorst Signed-off-by: Anshuman Gupta --- include/drm/drm_hdcp.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h index fe58dbb46962..ac22c246542a 100644 --- a/include/drm/drm_hdcp.h +++ b/include/drm/drm_hdcp.h @@ -101,11 +101,11 @@ /* Following Macros take a byte at a time for bit(s) masking */ /* - * TODO: This has to be changed for DP MST, as multiple stream on - * same port is possible. - * For HDCP2.2 on HDMI and DP SST this value is always 1. + * TODO: HDCP_2_2_MAX_CONTENT_STREAMS_CNT is based upon actual + * H/W MST streams capacity. + * This required to be moved out to platform specific header. */ -#define HDCP_2_2_MAX_CONTENT_STREAMS_CNT 1 +#define HDCP_2_2_MAX_CONTENT_STREAMS_CNT 4 #define HDCP_2_2_TXCAP_MASK_LEN2 #define HDCP_2_2_RXCAPS_LEN3 #define HDCP_2_2_RX_REPEATER(x)((x) & BIT(0)) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 08/16] drm/i915/hdcp: Pass dig_port to intel_hdcp_init
Pass dig_port as an argument to intel_hdcp_init()
and intel_hdcp2_init().
This will be required for HDCP 2.2 stream encryption.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 4 ++--
drivers/gpu/drm/i915/display/intel_hdcp.c| 12 +++-
drivers/gpu/drm/i915/display/intel_hdcp.h| 4 +++-
drivers/gpu/drm/i915/display/intel_hdmi.c| 2 +-
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
index 652d4645f255..384e384cb9e2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
@@ -751,10 +751,10 @@ int intel_dp_init_hdcp(struct intel_digital_port
*dig_port,
return 0;
if (intel_connector->mst_port)
- return intel_hdcp_init(intel_connector, port,
+ return intel_hdcp_init(intel_connector, dig_port,
_dp_mst_hdcp_shim);
else if (!intel_dp_is_edp(intel_dp))
- return intel_hdcp_init(intel_connector, port,
+ return intel_hdcp_init(intel_connector, dig_port,
_dp_hdcp_shim);
return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 46c9bd588db1..10770bf0e85e 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -1985,12 +1985,13 @@ static enum mei_fw_tc intel_get_mei_fw_tc(enum
transcoder cpu_transcoder)
}
static int initialize_hdcp_port_data(struct intel_connector *connector,
-enum port port,
+struct intel_digital_port *dig_port,
const struct intel_hdcp_shim *shim)
{
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
struct intel_hdcp *hdcp = >hdcp;
struct hdcp_port_data *data = >port_data;
+ enum port port = dig_port->base.port;
if (INTEL_GEN(dev_priv) < 12)
data->fw_ddi = intel_get_mei_fw_ddi_index(port);
@@ -2063,14 +2064,15 @@ void intel_hdcp_component_init(struct drm_i915_private
*dev_priv)
}
}
-static void intel_hdcp2_init(struct intel_connector *connector, enum port port,
+static void intel_hdcp2_init(struct intel_connector *connector,
+struct intel_digital_port *dig_port,
const struct intel_hdcp_shim *shim)
{
struct drm_i915_private *i915 = to_i915(connector->base.dev);
struct intel_hdcp *hdcp = >hdcp;
int ret;
- ret = initialize_hdcp_port_data(connector, port, shim);
+ ret = initialize_hdcp_port_data(connector, dig_port, shim);
if (ret) {
drm_dbg_kms(>drm, "Mei hdcp data init failed\n");
return;
@@ -2080,7 +2082,7 @@ static void intel_hdcp2_init(struct intel_connector
*connector, enum port port,
}
int intel_hdcp_init(struct intel_connector *connector,
- enum port port,
+ struct intel_digital_port *dig_port,
const struct intel_hdcp_shim *shim)
{
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
@@ -2091,7 +2093,7 @@ int intel_hdcp_init(struct intel_connector *connector,
return -EINVAL;
if (is_hdcp2_supported(dev_priv) && !connector->mst_port)
- intel_hdcp2_init(connector, port, shim);
+ intel_hdcp2_init(connector, dig_port, shim);
ret =
drm_connector_attach_content_protection_property(>base,
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h
b/drivers/gpu/drm/i915/display/intel_hdcp.h
index b912a3a0f5b8..8f53b0c7fe5c 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.h
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
@@ -18,13 +18,15 @@ struct intel_connector;
struct intel_crtc_state;
struct intel_encoder;
struct intel_hdcp_shim;
+struct intel_digital_port;
enum port;
enum transcoder;
void intel_hdcp_atomic_check(struct drm_connector *connector,
struct drm_connector_state *old_state,
struct drm_connector_state *new_state);
-int intel_hdcp_init(struct intel_connector *connector, enum port port,
+int intel_hdcp_init(struct intel_connector *connector,
+ struct intel_digital_port *dig_port,
const struct intel_hdcp_shim *hdcp_shim);
int intel_hdcp_enable(struct intel_connector *connector,
const struct intel_crtc_state *pipe_config, u8
content_type);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index f58469226694..0788de04711b 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@
[PATCH v3 09/16] drm/i915/hdcp: Encapsulate hdcp_port_data to dig_port
hdcp_port_data is specific to a port on which HDCP
encryption is getting enabled, so encapsulate it to
intel_digital_port.
This will be required to enable HDCP 2.2 stream encryption.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_ddi.c | 2 +
.../drm/i915/display/intel_display_types.h| 5 +-
drivers/gpu/drm/i915/display/intel_hdcp.c | 56 +++
3 files changed, 39 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index fbeffdfd1a0d..a46ba4e6a835 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4746,6 +4746,8 @@ static void intel_ddi_encoder_destroy(struct drm_encoder
*encoder)
intel_dp_encoder_flush_work(encoder);
drm_encoder_cleanup(encoder);
+ if (dig_port)
+ kfree(dig_port->port_data.streams);
kfree(dig_port);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 59b8fc21e3e8..749c3a7e0b45 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -402,7 +402,6 @@ struct intel_hdcp {
* content can flow only through a link protected by HDCP2.2.
*/
u8 content_type;
- struct hdcp_port_data port_data;
bool is_paired;
bool is_repeater;
@@ -1446,10 +1445,12 @@ struct intel_digital_port {
enum phy_fia tc_phy_fia;
u8 tc_phy_fia_idx;
- /* protects num_hdcp_streams reference count */
+ /* protects num_hdcp_streams reference count, port_data */
struct mutex hdcp_mutex;
/* the number of pipes using HDCP signalling out of this port */
unsigned int num_hdcp_streams;
+ /* HDCP port data need to pass to security f/w */
+ struct hdcp_port_data port_data;
void (*write_infoframe)(struct intel_encoder *encoder,
const struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 10770bf0e85e..207fa17129ae 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -15,6 +15,7 @@
#include
#include
+#include "i915_drv.h"
#include "i915_reg.h"
#include "intel_display_power.h"
#include "intel_display_types.h"
@@ -1031,7 +1032,8 @@ static int
hdcp2_prepare_ake_init(struct intel_connector *connector,
struct hdcp2_ake_init *ake_data)
{
- struct hdcp_port_data *data = >hdcp.port_data;
+ struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
+ struct hdcp_port_data *data = _port->port_data;
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
struct i915_hdcp_comp_master *comp;
int ret;
@@ -1060,7 +1062,8 @@ hdcp2_verify_rx_cert_prepare_km(struct intel_connector
*connector,
struct hdcp2_ake_no_stored_km *ek_pub_km,
size_t *msg_sz)
{
- struct hdcp_port_data *data = >hdcp.port_data;
+ struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
+ struct hdcp_port_data *data = _port->port_data;
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
struct i915_hdcp_comp_master *comp;
int ret;
@@ -1087,7 +1090,8 @@ hdcp2_verify_rx_cert_prepare_km(struct intel_connector
*connector,
static int hdcp2_verify_hprime(struct intel_connector *connector,
struct hdcp2_ake_send_hprime *rx_hprime)
{
- struct hdcp_port_data *data = >hdcp.port_data;
+ struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
+ struct hdcp_port_data *data = _port->port_data;
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
struct i915_hdcp_comp_master *comp;
int ret;
@@ -1112,7 +1116,8 @@ static int
hdcp2_store_pairing_info(struct intel_connector *connector,
struct hdcp2_ake_send_pairing_info *pairing_info)
{
- struct hdcp_port_data *data = >hdcp.port_data;
+ struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
+ struct hdcp_port_data *data = _port->port_data;
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
struct i915_hdcp_comp_master *comp;
int ret;
@@ -1138,7 +1143,8 @@ static int
hdcp2_prepare_lc_init(struct intel_connector *connector,
struct hdcp2_lc_init *lc_init)
{
- struct hdcp_port_data *data = >hdcp.port_data;
+ struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
+ struct hdcp_port_data *data = _port->port_data;
struct drm_i915_private *dev_priv =
[PATCH v3 01/16] drm/i915/hdcp: Update CP property in update_pipe
When crtc state need_modeset is true it is not necessary
it is going to be a real modeset, it can turns to be a
update_pipe instead of modeset.
This turns content protection property to be DESIRED and hdcp
update_pipe left with property to be in DESIRED state but
actually hdcp->value was ENABLED.
This caught with DP MST setup, when disabling HDCP on a connector
sets the crtc state need_modeset to true for all crtc driving
the other DP-MST topology connectors.
v2:
Fix WARN_ON(connector->base.registration_state == DRM_CONNECTOR_REGISTERED)
Fixes: 33f9a623bfc6 ("drm/i915/hdcp: Update CP as per the kernel internal
state")
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index b2a4bbcfdcd2..0d9e8d3b5603 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -2221,6 +2221,11 @@ void intel_hdcp_update_pipe(struct intel_atomic_state
*state,
desired_and_not_enabled =
hdcp->value != DRM_MODE_CONTENT_PROTECTION_ENABLED;
mutex_unlock(>mutex);
+
+ if (!desired_and_not_enabled &&
!content_protection_type_changed) {
+ drm_connector_get(>base);
+ schedule_work(>prop_work);
+ }
}
if (desired_and_not_enabled || content_protection_type_changed)
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 10/16] misc/mei/hdcp: Fix AUTH_STREAM_REQ cmd buffer len
Fix the size of WIRED_REPEATER_AUTH_STREAM_REQ cmd buffer size. It is based upon the actual number of MST streams and size of wired_cmd_repeater_auth_stream_req_in. Excluding the size of hdcp_cmd_header. v2: hdcp_cmd_header size annotation nitpick. [Tomas] Cc: Tomas Winkler Cc: Ramalingam C Acked-by: Tomas Winkler Signed-off-by: Anshuman Gupta --- drivers/misc/mei/hdcp/mei_hdcp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c index 9ae9669e46ea..3506a3534294 100644 --- a/drivers/misc/mei/hdcp/mei_hdcp.c +++ b/drivers/misc/mei/hdcp/mei_hdcp.c @@ -569,8 +569,7 @@ static int mei_hdcp_verify_mprime(struct device *dev, verify_mprime_in->header.api_version = HDCP_API_VERSION; verify_mprime_in->header.command_id = WIRED_REPEATER_AUTH_STREAM_REQ; verify_mprime_in->header.status = ME_HDCP_STATUS_SUCCESS; - verify_mprime_in->header.buffer_len = - WIRED_CMD_BUF_LEN_REPEATER_AUTH_STREAM_REQ_MIN_IN; + verify_mprime_in->header.buffer_len = cmd_size - sizeof(verify_mprime_in->header); verify_mprime_in->port.integrated_port_type = data->port_type; verify_mprime_in->port.physical_port = (u8)data->fw_ddi; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 03/16] drm/i915/hotplug: Handle CP_IRQ for DP-MST
Handle CP_IRQ in DEVICE_SERVICE_IRQ_VECTOR_ESI0
It requires to call intel_hdcp_handle_cp_irq() in case
of CP_IRQ is triggered by a sink in DP-MST topology.
Cc: "Ville Syrjälä"
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_dp.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 818daab252f3..21c6c9828cd7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5657,6 +5657,17 @@ static void intel_dp_handle_test_request(struct intel_dp
*intel_dp)
"Could not write test response to sink\n");
}
+static void
+intel_dp_mst_hpd_irq(struct intel_dp *intel_dp, u8 *esi, bool *handled)
+{
+ drm_dp_mst_hpd_irq(_dp->mst_mgr, esi, handled);
+
+ if (esi[1] & DP_CP_IRQ) {
+ intel_hdcp_handle_cp_irq(intel_dp->attached_connector);
+ *handled = true;
+ }
+}
+
/**
* intel_dp_check_mst_status - service any pending MST interrupts, check link
status
* @intel_dp: Intel DP struct
@@ -5701,7 +5712,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
drm_dbg_kms(>drm, "got esi %3ph\n", esi);
- drm_dp_mst_hpd_irq(_dp->mst_mgr, esi, );
+ intel_dp_mst_hpd_irq(intel_dp, esi, );
+
if (!handled)
break;
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 02/16] drm/i915/hdcp: Get conn while content_type changed
Get DRM connector reference count while scheduling a prop work
to avoid any possible destroy of DRM connector when it is in
DRM_CONNECTOR_REGISTERED state.
Fixes: a6597faa2d59 ("drm/i915: Protect workers against disappearing
connectors")
Cc: Sean Paul
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_hdcp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 0d9e8d3b5603..42cf91cf4f20 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -2210,6 +2210,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state
*state,
if (content_protection_type_changed) {
mutex_lock(>mutex);
hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
+ drm_connector_get(>base);
schedule_work(>prop_work);
mutex_unlock(>mutex);
}
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 07/16] drm/i915/hdcp: Enable Gen12 HDCP 1.4 DP MST support
Enable HDCP 1.4 over DP MST for Gen12.
This also enable the stream encryption support for
older generations, which was missing earlier.
v2:
- Added debug print for stream encryption.
- Disable the hdcp on port after disabling last stream
encryption.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 10 ++---
drivers/gpu/drm/i915/display/intel_hdcp.c | 46 ++---
2 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 16865b200062..f00e12fc83e8 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -826,13 +826,9 @@ static struct drm_connector
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
-
- /* TODO: Figure out how to make HDCP work on GEN12+ */
- if (INTEL_GEN(dev_priv) < 12) {
- ret = intel_dp_init_hdcp(dig_port, intel_connector);
- if (ret)
- DRM_DEBUG_KMS("HDCP init failed, skipping.\n");
- }
+ ret = intel_dp_init_hdcp(dig_port, intel_connector);
+ if (ret)
+ drm_dbg_kms(_priv->drm, "HDCP init failed, skipping.\n");
/*
* Reuse the prop from the SST connector because we're
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 61252d4be3dd..46c9bd588db1 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -612,7 +612,12 @@ int intel_hdcp_auth_downstream(struct intel_connector
*connector)
return ret;
}
-/* Implements Part 1 of the HDCP authorization procedure */
+/*
+ * Implements Part 1 of the HDCP authorization procedure.
+ * Authentication Part 1 steps for Multi-stream DisplayPort.
+ * Step 1. Auth Part 1 sequence on the driving MST Trasport Link.
+ * Step 2. Enable encryption for each stream that requires encryption.
+ */
static int intel_hdcp_auth(struct intel_connector *connector)
{
struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
@@ -766,10 +771,16 @@ static int intel_hdcp_auth(struct intel_connector
*connector)
return -ETIMEDOUT;
}
- /*
-* XXX: If we have MST-connected devices, we need to enable encryption
-* on those as well.
-*/
+ /* DP MST Auth Part 1 Step 2.a and Step 2.b */
+ if (shim->stream_encryption) {
+ ret = shim->stream_encryption(dig_port, true);
+ if (ret) {
+ drm_err(_priv->drm, "Failed to enable HDCP 1.4
stream enc\n");
+ return ret;
+ }
+ drm_dbg_kms(_priv->drm, "HDCP 1.4 tras %s stream
encrypted\n",
+ transcoder_name(hdcp->stream_transcoder));
+ }
if (repeater_present)
return intel_hdcp_auth_downstream(connector);
@@ -790,19 +801,26 @@ static int _intel_hdcp_disable(struct intel_connector
*connector)
drm_dbg_kms(_priv->drm, "[%s:%d] HDCP is being disabled...\n",
connector->base.name, connector->base.base.id);
+ /*
+* Step 1: Deselect HDCP Multiplestream Bit.
+* Step 2: poll for stream encryption status to be disable.
+*/
+ if (hdcp->shim->stream_encryption) {
+ ret = hdcp->shim->stream_encryption(dig_port, false);
+ if (ret) {
+ drm_err(_priv->drm, "Failed to disable HDCP 1.4
stream enc\n");
+ return ret;
+ }
+ drm_dbg_kms(_priv->drm, "HDCP 1.4 trans %s stream
encryption disabled\n",
+ transcoder_name(hdcp->stream_transcoder));
+ }
/*
-* If there are other connectors on this port using HDCP, don't disable
-* it. Instead, toggle the HDCP signalling off on that particular
-* connector/pipe and exit.
+* If there are other connectors on this port using HDCP, don't disable
it.
+* Repeat steps 1-2 for each stream that no longer requires encryption.
*/
- if (dig_port->num_hdcp_streams > 0) {
- ret = hdcp->shim->toggle_signalling(dig_port,
- cpu_transcoder, false);
- if (ret)
- DRM_ERROR("Failed to disable HDCP signalling\n");
+ if (dig_port->num_hdcp_streams > 0)
return ret;
- }
hdcp->hdcp_encrypted = false;
intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, port), 0);
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 04/16] drm/i915/hdcp: DP MST transcoder for link and stream
Gen12 has H/W delta with respect to HDCP{1.x,2.x} display engine
instances lies in Transcoder instead of DDI as in Gen11.
This requires hdcp driver to use mst_master_transcoder for link
authentication and stream transcoder for stream encryption
separately.
This will be used for both HDCP 1.4 and HDCP 2.2 over DP MST
on Gen12.
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
.../gpu/drm/i915/display/intel_display_types.h| 2 ++
drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdcp.c | 15 +++
drivers/gpu/drm/i915/display/intel_hdcp.h | 2 +-
5 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 09811be08cfe..bf8730267cfd 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4059,7 +4059,7 @@ static void intel_enable_ddi(struct intel_atomic_state
*state,
if (conn_state->content_protection ==
DRM_MODE_CONTENT_PROTECTION_DESIRED)
intel_hdcp_enable(to_intel_connector(conn_state->connector),
- crtc_state->cpu_transcoder,
+ crtc_state,
(u8)conn_state->hdcp_content_type);
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index f6f0626649e0..c47124a679b6 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -432,6 +432,8 @@ struct intel_hdcp {
* Hence caching the transcoder here.
*/
enum transcoder cpu_transcoder;
+ /* Only used for DP MST stream encryption */
+ enum transcoder stream_transcoder;
};
struct intel_connector {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index c8fcec4d0788..16865b200062 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -568,7 +568,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state
*state,
if (conn_state->content_protection ==
DRM_MODE_CONTENT_PROTECTION_DESIRED)
intel_hdcp_enable(to_intel_connector(conn_state->connector),
- pipe_config->cpu_transcoder,
+ pipe_config,
(u8)conn_state->hdcp_content_type);
}
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 42cf91cf4f20..a9b652c6e742 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -2095,7 +2095,7 @@ int intel_hdcp_init(struct intel_connector *connector,
}
int intel_hdcp_enable(struct intel_connector *connector,
- enum transcoder cpu_transcoder, u8 content_type)
+ const struct intel_crtc_state *pipe_config, u8
content_type)
{
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
struct intel_digital_port *dig_port =
intel_attached_dig_port(connector);
@@ -2111,10 +2111,17 @@ int intel_hdcp_enable(struct intel_connector *connector,
drm_WARN_ON(_priv->drm,
hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
hdcp->content_type = content_type;
- hdcp->cpu_transcoder = cpu_transcoder;
+
+ if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST)) {
+ hdcp->cpu_transcoder = pipe_config->mst_master_transcoder;
+ hdcp->stream_transcoder = pipe_config->cpu_transcoder;
+ } else {
+ hdcp->cpu_transcoder = pipe_config->cpu_transcoder;
+ hdcp->stream_transcoder = INVALID_TRANSCODER;
+ }
if (INTEL_GEN(dev_priv) >= 12)
- hdcp->port_data.fw_tc = intel_get_mei_fw_tc(cpu_transcoder);
+ hdcp->port_data.fw_tc =
intel_get_mei_fw_tc(hdcp->cpu_transcoder);
/*
* Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
@@ -2231,7 +2238,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state
*state,
if (desired_and_not_enabled || content_protection_type_changed)
intel_hdcp_enable(connector,
- crtc_state->cpu_transcoder,
+ crtc_state,
(u8)conn_state->hdcp_content_type);
}
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h
b/drivers/gpu/drm/i915/display/intel_hdcp.h
index 1bbf5b67ed0a..bc51c1e9b481 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.h
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
@@ -25,7 +25,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
int
[PATCH v3 05/16] drm/i915/hdcp: Move HDCP enc status timeout to header
DP MST stream encryption status requires time of a link frame
in order to change its status, but as there were some HDCP
encryption timeout observed earlier, it is safer to use
ENCRYPT_STATUS_CHANGE_TIMEOUT_MS timeout for stream status too,
it requires to move the macro to a header.
It will be used by both HDCP{1.x,2.x} stream status timeout.
Related: 7e90e8d0c0ea ("drm/i915: Increase timeout for Encrypt
status change")
Cc: Ramalingam C
Signed-off-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_hdcp.c | 9 -
drivers/gpu/drm/i915/display/intel_hdcp.h | 2 ++
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
b/drivers/gpu/drm/i915/display/intel_hdcp.c
index a9b652c6e742..61252d4be3dd 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -23,7 +23,6 @@
#include "intel_connector.h"
#define KEY_LOAD_TRIES 5
-#define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS 50
#define HDCP2_LC_RETRY_CNT 3
static
@@ -762,7 +761,7 @@ static int intel_hdcp_auth(struct intel_connector
*connector)
if (intel_de_wait_for_set(dev_priv,
HDCP_STATUS(dev_priv, cpu_transcoder, port),
HDCP_STATUS_ENC,
- ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
+ HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
drm_err(_priv->drm, "Timed out waiting for encryption\n");
return -ETIMEDOUT;
}
@@ -809,7 +808,7 @@ static int _intel_hdcp_disable(struct intel_connector
*connector)
intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, port), 0);
if (intel_de_wait_for_clear(dev_priv,
HDCP_STATUS(dev_priv, cpu_transcoder, port),
- ~0, ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) {
+ ~0, HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS))
{
drm_err(_priv->drm,
"Failed to disable HDCP, timeout clearing status\n");
return -ETIMEDOUT;
@@ -1641,7 +1640,7 @@ static int hdcp2_enable_encryption(struct intel_connector
*connector)
HDCP2_STATUS(dev_priv, cpu_transcoder,
port),
LINK_ENCRYPTION_STATUS,
- ENCRYPT_STATUS_CHANGE_TIMEOUT_MS);
+ HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS);
return ret;
}
@@ -1665,7 +1664,7 @@ static int hdcp2_disable_encryption(struct
intel_connector *connector)
HDCP2_STATUS(dev_priv, cpu_transcoder,
port),
LINK_ENCRYPTION_STATUS,
- ENCRYPT_STATUS_CHANGE_TIMEOUT_MS);
+ HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS);
if (ret == -ETIMEDOUT)
drm_dbg_kms(_priv->drm, "Disable Encryption Timedout");
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h
b/drivers/gpu/drm/i915/display/intel_hdcp.h
index bc51c1e9b481..b912a3a0f5b8 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.h
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
@@ -8,6 +8,8 @@
#include
+#define HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS 50
+
struct drm_connector;
struct drm_connector_state;
struct drm_i915_private;
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: Store USB device in struct drm_device
Hi On 22.10.20 11:20, Hans de Goede wrote: > Hi, > > On 10/21/20 3:07 PM, Thomas Zimmermann wrote: >> The drivers gm12u320 and udl operate on USB devices. They leave the >> PCI device in struct drm_device empty and store the USB device in their >> own driver structure. >> >> Fix this special case and save a few bytes by putting the USB device >> into an anonymous union with the PCI data. It's expected that DRM >> core and helpers only touch the PCI-device field for actual PCI devices. >> >> Thomas Zimmermann (3): >> drm: Add reference to USB device to struct drm_device >> drm/tiny/gm12u320: Store USB device in struct drm_device.udev >> drm/udl: Store USB device in struct drm_device.udev > > This series looks good to me: > > Reviewed-by: Hans de Goede Thanks a lot. Following Daniel's request, I'll drop patch 1 and instead do an upcast from drm_device.dev to the USB device structure. The driver patches 2 and 3 will be slightly different. Unless you object, I''ll take the r-b into the new patches. Best regards Thomas > > Regards, > > Hans > > >> >> drivers/gpu/drm/tiny/gm12u320.c | 52 + >> drivers/gpu/drm/udl/udl_connector.c | 8 ++--- >> drivers/gpu/drm/udl/udl_drv.c | 2 +- >> drivers/gpu/drm/udl/udl_drv.h | 1 - >> drivers/gpu/drm/udl/udl_main.c | 15 + >> include/drm/drm_device.h| 21 >> 6 files changed, 52 insertions(+), 47 deletions(-) >> >> -- >> 2.28.0 >> > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: Store USB device in struct drm_device
Hi, On 10/21/20 3:07 PM, Thomas Zimmermann wrote: > The drivers gm12u320 and udl operate on USB devices. They leave the > PCI device in struct drm_device empty and store the USB device in their > own driver structure. > > Fix this special case and save a few bytes by putting the USB device > into an anonymous union with the PCI data. It's expected that DRM > core and helpers only touch the PCI-device field for actual PCI devices. > > Thomas Zimmermann (3): > drm: Add reference to USB device to struct drm_device > drm/tiny/gm12u320: Store USB device in struct drm_device.udev > drm/udl: Store USB device in struct drm_device.udev This series looks good to me: Reviewed-by: Hans de Goede Regards, Hans > > drivers/gpu/drm/tiny/gm12u320.c | 52 + > drivers/gpu/drm/udl/udl_connector.c | 8 ++--- > drivers/gpu/drm/udl/udl_drv.c | 2 +- > drivers/gpu/drm/udl/udl_drv.h | 1 - > drivers/gpu/drm/udl/udl_main.c | 15 + > include/drm/drm_device.h| 21 > 6 files changed, 52 insertions(+), 47 deletions(-) > > -- > 2.28.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 08/10] drm/gem: Store client buffer mappings as struct dma_buf_map
Hi
On 22.10.20 10:49, Daniel Vetter wrote:
> On Tue, Oct 20, 2020 at 02:20:44PM +0200, Thomas Zimmermann wrote:
>> Kernel DRM clients now store their framebuffer address in an instance
>> of struct dma_buf_map. Depending on the buffer's location, the address
>> refers to system or I/O memory.
>>
>> Callers of drm_client_buffer_vmap() receive a copy of the value in
>> the call's supplied arguments. It can be accessed and modified with
>> dma_buf_map interfaces.
>>
>> Signed-off-by: Thomas Zimmermann
>> Reviewed-by: Daniel Vetter
>> Tested-by: Sam Ravnborg
>> ---
>> drivers/gpu/drm/drm_client.c| 34 +++--
>> drivers/gpu/drm/drm_fb_helper.c | 23 +-
>> include/drm/drm_client.h| 7 ---
>> 3 files changed, 38 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
>> index ac0082bed966..fe573acf1067 100644
>> --- a/drivers/gpu/drm/drm_client.c
>> +++ b/drivers/gpu/drm/drm_client.c
>> @@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
>> drm_client_buffer *buffer)
>> {
>> struct drm_device *dev = buffer->client->dev;
>>
>> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
>> +drm_gem_vunmap(buffer->gem, >map);
>>
>> if (buffer->gem)
>> drm_gem_object_put(buffer->gem);
>> @@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev
>> *client, u32 width, u32 height, u
>> /**
>> * drm_client_buffer_vmap - Map DRM client buffer into address space
>> * @buffer: DRM client buffer
>> + * @map_copy: Returns the mapped memory's address
>> *
>> * This function maps a client buffer into kernel address space. If the
>> - * buffer is already mapped, it returns the mapping's address.
>> + * buffer is already mapped, it returns the existing mapping's address.
>> *
>> * Client buffer mappings are not ref'counted. Each call to
>> * drm_client_buffer_vmap() should be followed by a call to
>> * drm_client_buffer_vunmap(); or the client buffer should be mapped
>> * throughout its lifetime.
>> *
>> + * The returned address is a copy of the internal value. In contrast to
>> + * other vmap interfaces, you don't need it for the client's vunmap
>> + * function. So you can modify it at will during blit and draw operations.
>> + *
>> * Returns:
>> - * The mapped memory's address
>> + * 0 on success, or a negative errno code otherwise.
>> */
>> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
>> +int
>> +drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map
>> *map_copy)
>> {
>> -struct dma_buf_map map;
>> +struct dma_buf_map *map = >map;
>> int ret;
>>
>> -if (buffer->vaddr)
>> -return buffer->vaddr;
>> +if (dma_buf_map_is_set(map))
>> +goto out;
>>
>> /*
>> * FIXME: The dependency on GEM here isn't required, we could
>> @@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
>> *buffer)
>> * fd_install step out of the driver backend hooks, to make that
>> * final step optional for internal users.
>> */
>> -ret = drm_gem_vmap(buffer->gem, );
>> +ret = drm_gem_vmap(buffer->gem, map);
>> if (ret)
>> -return ERR_PTR(ret);
>> +return ret;
>>
>> -buffer->vaddr = map.vaddr;
>> +out:
>> +*map_copy = *map;
>>
>> -return map.vaddr;
>> +return 0;
>> }
>> EXPORT_SYMBOL(drm_client_buffer_vmap);
>>
>> @@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
>> */
>> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
>> {
>> -struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
>> +struct dma_buf_map *map = >map;
>>
>> -drm_gem_vunmap(buffer->gem, );
>> -buffer->vaddr = NULL;
>> +drm_gem_vunmap(buffer->gem, map);
>> }
>> EXPORT_SYMBOL(drm_client_buffer_vunmap);
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index c2f72bb6afb1..6212cd7cde1d 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -378,7 +378,7 @@ static void drm_fb_helper_dirty_blit_real(struct
>> drm_fb_helper *fb_helper,
>> unsigned int cpp = fb->format->cpp[0];
>> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
>> void *src = fb_helper->fbdev->screen_buffer + offset;
>> -void *dst = fb_helper->buffer->vaddr + offset;
>> +void *dst = fb_helper->buffer->map.vaddr + offset;
>> size_t len = (clip->x2 - clip->x1) * cpp;
>> unsigned int y;
>>
>> @@ -400,7 +400,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
>> *work)
>> struct drm_clip_rect *clip = >dirty_clip;
>> struct drm_clip_rect clip_copy;
>> unsigned long flags;
>> -void *vaddr;
>> +struct dma_buf_map map;
>> +int ret;
>>
>> spin_lock_irqsave(>dirty_lock, flags);
>> clip_copy = *clip;
>> @@ -413,8
Re: [PATCH v5 10/10] drm/fb_helper: Support framebuffers in I/O memory
On Thu, Oct 22, 2020 at 10:37:56AM +0200, Thomas Zimmermann wrote:
> Hi
>
> On 22.10.20 10:05, Daniel Vetter wrote:
> > On Tue, Oct 20, 2020 at 02:20:46PM +0200, Thomas Zimmermann wrote:
> >> At least sparc64 requires I/O-specific access to framebuffers. This
> >> patch updates the fbdev console accordingly.
> >>
> >> For drivers with direct access to the framebuffer memory, the callback
> >> functions in struct fb_ops test for the type of memory and call the rsp
> >> fb_sys_ of fb_cfb_ functions. Read and write operations are implemented
> >> internally by DRM's fbdev helper.
> >>
> >> For drivers that employ a shadow buffer, fbdev's blit function retrieves
> >> the framebuffer address as struct dma_buf_map, and uses dma_buf_map
> >> interfaces to access the buffer.
> >>
> >> The bochs driver on sparc64 uses a workaround to flag the framebuffer as
> >> I/O memory and avoid a HW exception. With the introduction of struct
> >> dma_buf_map, this is not required any longer. The patch removes the rsp
> >> code from both, bochs and fbdev.
> >>
> >> v5:
> >>* implement fb_read/fb_write internally (Daniel, Sam)
> >> v4:
> >>* move dma_buf_map changes into separate patch (Daniel)
> >>* TODO list: comment on fbdev updates (Daniel)
> >>
> >> Signed-off-by: Thomas Zimmermann
> >> Tested-by: Sam Ravnborg
> >> ---
> >> Documentation/gpu/todo.rst| 19 ++-
> >> drivers/gpu/drm/bochs/bochs_kms.c | 1 -
> >> drivers/gpu/drm/drm_fb_helper.c | 227 --
> >> include/drm/drm_mode_config.h | 12 --
> >> 4 files changed, 230 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> >> index 7e6fc3c04add..638b7f704339 100644
> >> --- a/Documentation/gpu/todo.rst
> >> +++ b/Documentation/gpu/todo.rst
> >> @@ -197,13 +197,28 @@ Convert drivers to use drm_fbdev_generic_setup()
> >>
> >>
> >> Most drivers can use drm_fbdev_generic_setup(). Driver have to implement
> >> -atomic modesetting and GEM vmap support. Current generic fbdev emulation
> >> -expects the framebuffer in system memory (or system-like memory).
> >> +atomic modesetting and GEM vmap support. Historically, generic fbdev
> >> emulation
> >> +expected the framebuffer in system memory or system-like memory. By
> >> employing
> >> +struct dma_buf_map, drivers with frambuffers in I/O memory can be
> >> supported
> >> +as well.
> >>
> >> Contact: Maintainer of the driver you plan to convert
> >>
> >> Level: Intermediate
> >>
> >> +Reimplement functions in drm_fbdev_fb_ops without fbdev
> >> +---
> >> +
> >> +A number of callback functions in drm_fbdev_fb_ops could benefit from
> >> +being rewritten without dependencies on the fbdev module. Some of the
> >> +helpers could further benefit from using struct dma_buf_map instead of
> >> +raw pointers.
> >> +
> >> +Contact: Thomas Zimmermann , Daniel Vetter
> >> +
> >> +Level: Advanced
> >> +
> >> +
> >> drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup
> >> -
> >>
> >> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
> >> b/drivers/gpu/drm/bochs/bochs_kms.c
> >> index 13d0d04c4457..853081d186d5 100644
> >> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> >> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> >> @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
> >>bochs->dev->mode_config.preferred_depth = 24;
> >>bochs->dev->mode_config.prefer_shadow = 0;
> >>bochs->dev->mode_config.prefer_shadow_fbdev = 1;
> >> - bochs->dev->mode_config.fbdev_use_iomem = true;
> >>bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
> >>
> >>bochs->dev->mode_config.funcs = _mode_funcs;
> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> >> b/drivers/gpu/drm/drm_fb_helper.c
> >> index 6212cd7cde1d..1d3180841778 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -372,24 +372,22 @@ static void drm_fb_helper_resume_worker(struct
> >> work_struct *work)
> >> }
> >>
> >> static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
> >> -struct drm_clip_rect *clip)
> >> +struct drm_clip_rect *clip,
> >> +struct dma_buf_map *dst)
> >> {
> >>struct drm_framebuffer *fb = fb_helper->fb;
> >>unsigned int cpp = fb->format->cpp[0];
> >>size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> >>void *src = fb_helper->fbdev->screen_buffer + offset;
> >> - void *dst = fb_helper->buffer->map.vaddr + offset;
> >>size_t len = (clip->x2 - clip->x1) * cpp;
> >>unsigned int y;
> >>
> >> - for (y = clip->y1; y < clip->y2; y++) {
> >> - if (!fb_helper->dev->mode_config.fbdev_use_iomem)
> >> -
Re: [PATCH v5 08/10] drm/gem: Store client buffer mappings as struct dma_buf_map
On Tue, Oct 20, 2020 at 02:20:44PM +0200, Thomas Zimmermann wrote:
> Kernel DRM clients now store their framebuffer address in an instance
> of struct dma_buf_map. Depending on the buffer's location, the address
> refers to system or I/O memory.
>
> Callers of drm_client_buffer_vmap() receive a copy of the value in
> the call's supplied arguments. It can be accessed and modified with
> dma_buf_map interfaces.
>
> Signed-off-by: Thomas Zimmermann
> Reviewed-by: Daniel Vetter
> Tested-by: Sam Ravnborg
> ---
> drivers/gpu/drm/drm_client.c| 34 +++--
> drivers/gpu/drm/drm_fb_helper.c | 23 +-
> include/drm/drm_client.h| 7 ---
> 3 files changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index ac0082bed966..fe573acf1067 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
> drm_client_buffer *buffer)
> {
> struct drm_device *dev = buffer->client->dev;
>
> - drm_gem_vunmap(buffer->gem, buffer->vaddr);
> + drm_gem_vunmap(buffer->gem, >map);
>
> if (buffer->gem)
> drm_gem_object_put(buffer->gem);
> @@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev *client,
> u32 width, u32 height, u
> /**
> * drm_client_buffer_vmap - Map DRM client buffer into address space
> * @buffer: DRM client buffer
> + * @map_copy: Returns the mapped memory's address
> *
> * This function maps a client buffer into kernel address space. If the
> - * buffer is already mapped, it returns the mapping's address.
> + * buffer is already mapped, it returns the existing mapping's address.
> *
> * Client buffer mappings are not ref'counted. Each call to
> * drm_client_buffer_vmap() should be followed by a call to
> * drm_client_buffer_vunmap(); or the client buffer should be mapped
> * throughout its lifetime.
> *
> + * The returned address is a copy of the internal value. In contrast to
> + * other vmap interfaces, you don't need it for the client's vunmap
> + * function. So you can modify it at will during blit and draw operations.
> + *
> * Returns:
> - * The mapped memory's address
> + * 0 on success, or a negative errno code otherwise.
> */
> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> +int
> +drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map
> *map_copy)
> {
> - struct dma_buf_map map;
> + struct dma_buf_map *map = >map;
> int ret;
>
> - if (buffer->vaddr)
> - return buffer->vaddr;
> + if (dma_buf_map_is_set(map))
> + goto out;
>
> /*
>* FIXME: The dependency on GEM here isn't required, we could
> @@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
> *buffer)
>* fd_install step out of the driver backend hooks, to make that
>* final step optional for internal users.
>*/
> - ret = drm_gem_vmap(buffer->gem, );
> + ret = drm_gem_vmap(buffer->gem, map);
> if (ret)
> - return ERR_PTR(ret);
> + return ret;
>
> - buffer->vaddr = map.vaddr;
> +out:
> + *map_copy = *map;
>
> - return map.vaddr;
> + return 0;
> }
> EXPORT_SYMBOL(drm_client_buffer_vmap);
>
> @@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
> */
> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> {
> - struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
> + struct dma_buf_map *map = >map;
>
> - drm_gem_vunmap(buffer->gem, );
> - buffer->vaddr = NULL;
> + drm_gem_vunmap(buffer->gem, map);
> }
> EXPORT_SYMBOL(drm_client_buffer_vunmap);
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index c2f72bb6afb1..6212cd7cde1d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -378,7 +378,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> drm_fb_helper *fb_helper,
> unsigned int cpp = fb->format->cpp[0];
> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> void *src = fb_helper->fbdev->screen_buffer + offset;
> - void *dst = fb_helper->buffer->vaddr + offset;
> + void *dst = fb_helper->buffer->map.vaddr + offset;
> size_t len = (clip->x2 - clip->x1) * cpp;
> unsigned int y;
>
> @@ -400,7 +400,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
> struct drm_clip_rect *clip = >dirty_clip;
> struct drm_clip_rect clip_copy;
> unsigned long flags;
> - void *vaddr;
> + struct dma_buf_map map;
> + int ret;
>
> spin_lock_irqsave(>dirty_lock, flags);
> clip_copy = *clip;
> @@ -413,8 +414,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
>
> /* Generic fbdev uses a shadow
Re: [PATCH v5 10/10] drm/fb_helper: Support framebuffers in I/O memory
Hi
On 22.10.20 10:05, Daniel Vetter wrote:
> On Tue, Oct 20, 2020 at 02:20:46PM +0200, Thomas Zimmermann wrote:
>> At least sparc64 requires I/O-specific access to framebuffers. This
>> patch updates the fbdev console accordingly.
>>
>> For drivers with direct access to the framebuffer memory, the callback
>> functions in struct fb_ops test for the type of memory and call the rsp
>> fb_sys_ of fb_cfb_ functions. Read and write operations are implemented
>> internally by DRM's fbdev helper.
>>
>> For drivers that employ a shadow buffer, fbdev's blit function retrieves
>> the framebuffer address as struct dma_buf_map, and uses dma_buf_map
>> interfaces to access the buffer.
>>
>> The bochs driver on sparc64 uses a workaround to flag the framebuffer as
>> I/O memory and avoid a HW exception. With the introduction of struct
>> dma_buf_map, this is not required any longer. The patch removes the rsp
>> code from both, bochs and fbdev.
>>
>> v5:
>> * implement fb_read/fb_write internally (Daniel, Sam)
>> v4:
>> * move dma_buf_map changes into separate patch (Daniel)
>> * TODO list: comment on fbdev updates (Daniel)
>>
>> Signed-off-by: Thomas Zimmermann
>> Tested-by: Sam Ravnborg
>> ---
>> Documentation/gpu/todo.rst| 19 ++-
>> drivers/gpu/drm/bochs/bochs_kms.c | 1 -
>> drivers/gpu/drm/drm_fb_helper.c | 227 --
>> include/drm/drm_mode_config.h | 12 --
>> 4 files changed, 230 insertions(+), 29 deletions(-)
>>
>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 7e6fc3c04add..638b7f704339 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -197,13 +197,28 @@ Convert drivers to use drm_fbdev_generic_setup()
>>
>>
>> Most drivers can use drm_fbdev_generic_setup(). Driver have to implement
>> -atomic modesetting and GEM vmap support. Current generic fbdev emulation
>> -expects the framebuffer in system memory (or system-like memory).
>> +atomic modesetting and GEM vmap support. Historically, generic fbdev
>> emulation
>> +expected the framebuffer in system memory or system-like memory. By
>> employing
>> +struct dma_buf_map, drivers with frambuffers in I/O memory can be supported
>> +as well.
>>
>> Contact: Maintainer of the driver you plan to convert
>>
>> Level: Intermediate
>>
>> +Reimplement functions in drm_fbdev_fb_ops without fbdev
>> +---
>> +
>> +A number of callback functions in drm_fbdev_fb_ops could benefit from
>> +being rewritten without dependencies on the fbdev module. Some of the
>> +helpers could further benefit from using struct dma_buf_map instead of
>> +raw pointers.
>> +
>> +Contact: Thomas Zimmermann , Daniel Vetter
>> +
>> +Level: Advanced
>> +
>> +
>> drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup
>> -
>>
>> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
>> b/drivers/gpu/drm/bochs/bochs_kms.c
>> index 13d0d04c4457..853081d186d5 100644
>> --- a/drivers/gpu/drm/bochs/bochs_kms.c
>> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
>> @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
>> bochs->dev->mode_config.preferred_depth = 24;
>> bochs->dev->mode_config.prefer_shadow = 0;
>> bochs->dev->mode_config.prefer_shadow_fbdev = 1;
>> -bochs->dev->mode_config.fbdev_use_iomem = true;
>> bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
>>
>> bochs->dev->mode_config.funcs = _mode_funcs;
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 6212cd7cde1d..1d3180841778 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -372,24 +372,22 @@ static void drm_fb_helper_resume_worker(struct
>> work_struct *work)
>> }
>>
>> static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
>> - struct drm_clip_rect *clip)
>> + struct drm_clip_rect *clip,
>> + struct dma_buf_map *dst)
>> {
>> struct drm_framebuffer *fb = fb_helper->fb;
>> unsigned int cpp = fb->format->cpp[0];
>> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
>> void *src = fb_helper->fbdev->screen_buffer + offset;
>> -void *dst = fb_helper->buffer->map.vaddr + offset;
>> size_t len = (clip->x2 - clip->x1) * cpp;
>> unsigned int y;
>>
>> -for (y = clip->y1; y < clip->y2; y++) {
>> -if (!fb_helper->dev->mode_config.fbdev_use_iomem)
>> -memcpy(dst, src, len);
>> -else
>> -memcpy_toio((void __iomem *)dst, src, len);
>> +dma_buf_map_incr(dst, offset); /* go to first pixel within clip rect */
>>
>> +for (y = clip->y1; y < clip->y2; y++) {
>>
Re: [PATCH 2/4] disp/msm/dpu: add support to dump dpu registers
Hi Abhinav,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-exynos/exynos-drm-next]
[also build test WARNING on drm-intel/for-linux-next
tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.9 next-20201022]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507
base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
exynos-drm-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/a7e6907c303a46ea8422fc3c414c22fdfb45d49f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507
git checkout a7e6907c303a46ea8422fc3c414c22fdfb45d49f
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c: In function 'dpu_dbg_dump':
>> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c:115:6: warning: variable 'i' set but
>> not used [-Wunused-but-set-variable]
115 | int i, index = 0;
| ^
vim +/i +115 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c
112
113 void dpu_dbg_dump(enum dpu_dbg_dump_context dump_mode, const char
*name, ...)
114 {
> 115 int i, index = 0;
116 bool do_panic = false;
117 bool dump_all = false;
118 va_list args;
119 char *blk_name = NULL;
120 struct dpu_dbg_reg_base *blk_base = NULL;
121 struct dpu_dbg_reg_base **blk_arr;
122 u32 blk_len;
123
124 /*
125 * if there is a coredump pending return immediately till dump
126 * if read by userspace or timeout happens
127 */
128 if (((dpu_dbg.enable_reg_dump == DPU_DBG_DUMP_IN_MEM) ||
129 (dpu_dbg.enable_reg_dump == DPU_DBG_DUMP_IN_COREDUMP))
&&
130 dpu_dbg.coredump_pending) {
131 pr_debug("coredump is pending read\n");
132 return;
133 }
134
135 blk_arr = _dbg.req_dump_blks[0];
136 blk_len = ARRAY_SIZE(dpu_dbg.req_dump_blks);
137
138 memset(dpu_dbg.req_dump_blks, 0,
139 sizeof(dpu_dbg.req_dump_blks));
140 dpu_dbg.dump_all = false;
141 dpu_dbg.dump_mode = dump_mode;
142
143 va_start(args, name);
144 i = 0;
145 while ((blk_name = va_arg(args, char*))) {
146
147 if (IS_ERR_OR_NULL(blk_name))
148 break;
149
150 blk_base = _dpu_dump_get_blk_addr(_dbg, blk_name);
151 if (blk_base) {
152 if (index < blk_len) {
153 blk_arr[index] = blk_base;
154 index++;
155 } else {
156 pr_err("insufficient space to dump
%s\n",
157 blk_name);
158 }
159 }
160
161 if (!strcmp(blk_name, "all"))
162 dump_all = true;
163
164 if (!strcmp(blk_name, "panic"))
165 do_panic = true;
166
167 }
168 va_end(args);
169
170 dpu_dbg.work_panic = do_panic;
171 dpu_dbg.dump_all = dump_all;
172
173 kthread_queue_work(dpu_dbg.dump_worker,
174 _dbg.dump_work);
175
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: remove overlapping memcpy support
Am 22.10.20 um 05:11 schrieb Dave Airlie:
From: Dave Airlie
remove the overlapping memcp support as it's never used.
Signed-off-by: Dave Airlie
Reviewed-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 0a5694ef1e07..ecb54415d1ca 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -180,9 +180,6 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
void *new_iomap;
int ret;
unsigned long i;
- unsigned long page;
- unsigned long add = 0;
- int dir;
ret = ttm_bo_wait_ctx(bo, ctx);
if (ret)
@@ -220,27 +217,17 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
goto out1;
}
- add = 0;
- dir = 1;
-
- if ((old_mem->mem_type == new_mem->mem_type) &&
- (new_mem->start < old_mem->start + old_mem->size)) {
- dir = -1;
- add = new_mem->num_pages - 1;
- }
-
for (i = 0; i < new_mem->num_pages; ++i) {
- page = i * dir + add;
if (old_iomap == NULL) {
pgprot_t prot = ttm_io_prot(bo, old_mem, PAGE_KERNEL);
- ret = ttm_copy_ttm_io_page(ttm, new_iomap, page,
+ ret = ttm_copy_ttm_io_page(ttm, new_iomap, i,
prot);
} else if (new_iomap == NULL) {
pgprot_t prot = ttm_io_prot(bo, new_mem, PAGE_KERNEL);
- ret = ttm_copy_io_ttm_page(ttm, old_iomap, page,
+ ret = ttm_copy_io_ttm_page(ttm, old_iomap, i,
prot);
} else {
- ret = ttm_copy_io_page(new_iomap, old_iomap, page);
+ ret = ttm_copy_io_page(new_iomap, old_iomap, i);
}
if (ret)
goto out1;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 10/10] drm/fb_helper: Support framebuffers in I/O memory
On Tue, Oct 20, 2020 at 02:20:46PM +0200, Thomas Zimmermann wrote:
> At least sparc64 requires I/O-specific access to framebuffers. This
> patch updates the fbdev console accordingly.
>
> For drivers with direct access to the framebuffer memory, the callback
> functions in struct fb_ops test for the type of memory and call the rsp
> fb_sys_ of fb_cfb_ functions. Read and write operations are implemented
> internally by DRM's fbdev helper.
>
> For drivers that employ a shadow buffer, fbdev's blit function retrieves
> the framebuffer address as struct dma_buf_map, and uses dma_buf_map
> interfaces to access the buffer.
>
> The bochs driver on sparc64 uses a workaround to flag the framebuffer as
> I/O memory and avoid a HW exception. With the introduction of struct
> dma_buf_map, this is not required any longer. The patch removes the rsp
> code from both, bochs and fbdev.
>
> v5:
> * implement fb_read/fb_write internally (Daniel, Sam)
> v4:
> * move dma_buf_map changes into separate patch (Daniel)
> * TODO list: comment on fbdev updates (Daniel)
>
> Signed-off-by: Thomas Zimmermann
> Tested-by: Sam Ravnborg
> ---
> Documentation/gpu/todo.rst| 19 ++-
> drivers/gpu/drm/bochs/bochs_kms.c | 1 -
> drivers/gpu/drm/drm_fb_helper.c | 227 --
> include/drm/drm_mode_config.h | 12 --
> 4 files changed, 230 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 7e6fc3c04add..638b7f704339 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -197,13 +197,28 @@ Convert drivers to use drm_fbdev_generic_setup()
>
>
> Most drivers can use drm_fbdev_generic_setup(). Driver have to implement
> -atomic modesetting and GEM vmap support. Current generic fbdev emulation
> -expects the framebuffer in system memory (or system-like memory).
> +atomic modesetting and GEM vmap support. Historically, generic fbdev
> emulation
> +expected the framebuffer in system memory or system-like memory. By employing
> +struct dma_buf_map, drivers with frambuffers in I/O memory can be supported
> +as well.
>
> Contact: Maintainer of the driver you plan to convert
>
> Level: Intermediate
>
> +Reimplement functions in drm_fbdev_fb_ops without fbdev
> +---
> +
> +A number of callback functions in drm_fbdev_fb_ops could benefit from
> +being rewritten without dependencies on the fbdev module. Some of the
> +helpers could further benefit from using struct dma_buf_map instead of
> +raw pointers.
> +
> +Contact: Thomas Zimmermann , Daniel Vetter
> +
> +Level: Advanced
> +
> +
> drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup
> -
>
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
> b/drivers/gpu/drm/bochs/bochs_kms.c
> index 13d0d04c4457..853081d186d5 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
> bochs->dev->mode_config.preferred_depth = 24;
> bochs->dev->mode_config.prefer_shadow = 0;
> bochs->dev->mode_config.prefer_shadow_fbdev = 1;
> - bochs->dev->mode_config.fbdev_use_iomem = true;
> bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
>
> bochs->dev->mode_config.funcs = _mode_funcs;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 6212cd7cde1d..1d3180841778 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -372,24 +372,22 @@ static void drm_fb_helper_resume_worker(struct
> work_struct *work)
> }
>
> static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
> - struct drm_clip_rect *clip)
> + struct drm_clip_rect *clip,
> + struct dma_buf_map *dst)
> {
> struct drm_framebuffer *fb = fb_helper->fb;
> unsigned int cpp = fb->format->cpp[0];
> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> void *src = fb_helper->fbdev->screen_buffer + offset;
> - void *dst = fb_helper->buffer->map.vaddr + offset;
> size_t len = (clip->x2 - clip->x1) * cpp;
> unsigned int y;
>
> - for (y = clip->y1; y < clip->y2; y++) {
> - if (!fb_helper->dev->mode_config.fbdev_use_iomem)
> - memcpy(dst, src, len);
> - else
> - memcpy_toio((void __iomem *)dst, src, len);
> + dma_buf_map_incr(dst, offset); /* go to first pixel within clip rect */
>
> + for (y = clip->y1; y < clip->y2; y++) {
> + dma_buf_map_memcpy_to(dst, src, len);
> + dma_buf_map_incr(dst, fb->pitches[0]);
> src +=
Re: [PATCH v2 08/17] s390/pci: Remove races against pte updates
On Wed, Oct 21, 2020 at 09:55:57AM +0200, Niklas Schnelle wrote:
> Hi Daniel,
>
> friendly ping. I haven't seen a new version of this patch series,
> as I said I think your change for s390/pci is generally useful so
> I'm curious, are you planning on sending a new version soon?
> If you want you can also just sent this patch with the last few
> nitpicks (primarily the mail address) fixed and I'll happily apply.
(I think this was stuck somewhere in moderation, only showed up just now)
I was waiting for the testing result for the habana driver from Oded, but
I guess Oded was waiting for v3. Hence the delay.
Cheers, Daniel
>
> Best regards,
> Niklas Schnelle
>
> On 10/12/20 4:19 PM, Daniel Vetter wrote:
> > On Mon, Oct 12, 2020 at 04:03:28PM +0200, Niklas Schnelle wrote:
> ... snip
> >>> Cc: Jason Gunthorpe
> >>> Cc: Dan Williams
> >>> Cc: Kees Cook
> >>> Cc: Andrew Morton
> >>> Cc: John Hubbard
> >>> Cc: Jérôme Glisse
> >>> Cc: Jan Kara
> >>> Cc: Dan Williams
> >>
> >> The above Cc: line for Dan Williams is a duplicate
> >>
> >>> Cc: linux...@kvack.org
> >>> Cc: linux-arm-ker...@lists.infradead.org
> >>> Cc: linux-samsung-...@vger.kernel.org
> >>> Cc: linux-me...@vger.kernel.org
> >>> Cc: Niklas Schnelle
> >>> Cc: Gerald Schaefer
> >>> Cc: linux-s...@vger.kernel.org
> >>> --
> >>> v2: Move VM_IO | VM_PFNMAP checks around so they keep returning EINVAL
> >>> like before (Gerard)
> >>
> >> I think the above should go before the CC/Signed-off/Reviewev block.
> >
> > This is a per-subsystem bikeshed :-) drivers/gpu definitely wants it
> > above, but most core subsystems want it below. I'll move it.
> >
> >>> ---
> >>> arch/s390/pci/pci_mmio.c | 98 +++-
> >>> 1 file changed, 57 insertions(+), 41 deletions(-)
> >>>
> >>> diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
> >>> index 401cf670a243..1a6adbc68ee8 100644
> >>> --- a/arch/s390/pci/pci_mmio.c
> >>> +++ b/arch/s390/pci/pci_mmio.c
> >>> @@ -119,33 +119,15 @@ static inline int __memcpy_toio_inuser(void __iomem
> >>> *dst,
> >>> return rc;
> >>> }
> >>>
> >>> -static long get_pfn(unsigned long user_addr, unsigned long access,
> >>> - unsigned long *pfn)
> >>> -{
> >>> - struct vm_area_struct *vma;
> >>> - long ret;
> >>> -
> >>> - mmap_read_lock(current->mm);
> >>> - ret = -EINVAL;
> >>> - vma = find_vma(current->mm, user_addr);
> >>> - if (!vma)
> >>> - goto out;
> >>> - ret = -EACCES;
> >>> - if (!(vma->vm_flags & access))
> >>> - goto out;
> >>> - ret = follow_pfn(vma, user_addr, pfn);
> >>> -out:
> >>> - mmap_read_unlock(current->mm);
> >>> - return ret;
> >>> -}
> >>> -
> >>> SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
> >>> const void __user *, user_buffer, size_t, length)
> >>> {
> >>> u8 local_buf[64];
> >>> void __iomem *io_addr;
> >>> void *buf;
> >>> - unsigned long pfn;
> >>> + struct vm_area_struct *vma;
> >>> + pte_t *ptep;
> >>> + spinlock_t *ptl;
> >>
> >> With checkpatch.pl --strict the above yields a complained
> >> "CHECK: spinlock_t definition without comment" but I think
> >> that's really okay since your commit description is very clear.
> >> Same oin line 277.
> >
> > I think this is a falls positive, checkpatch doesn't realize that
> > SYSCALL_DEFINE3 is a function, not a structure. And in a structure I'd
> > have added the kerneldoc or comment.
> >
> > I'll fix up all the nits you've found for the next round. Thanks for
> > taking a look.
> > -Daniel
> >
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: add client cap to expose low power modes
On Wed, 21 Oct 2020 18:09:05 +0100 Daniel Stone wrote: > On Wed, 21 Oct 2020 at 17:34, Daniel Vetter wrote: > > On Wed, Oct 21, 2020 at 05:11:00PM +0100, Daniel Stone wrote: > > > It makes sense to me: some modes are annotated with a 'low-power' > > > flag, tucked away behind a client cap which makes clients opt in, and > > > they can switch into the low-power mode (letting the display/panel > > > save a lot of power) _if_ they only have at most 15% of pixels lit up. > > > > > > My worry is about the 15% though ... what happens when hardware allows > > > up to 20%, or only allows 10%? > > > > Yeah exactly, that's what I'm worried about too, these kind of details. > > Opt-in flag for special modes, no problem, but we need to make sure we > > agree on what flavour of special exactly they are. Hi, I second those concerns. I would also like to hear the reason why low power should be tied to a video mode instead of being an orthogonal property. Does low power depend on different timings? Different resolution? Maybe extremely low refresh rate plays a role here? Or maybe it goes into panel self-refresh mode that is somehow different from normal timing wise? How does low power mode interact with backlight controls? Does low power mode automatically change the backlight control range, or the control value, or does userspace need to dim down the backlight explicitly, or what should happen? What if userspace does not do what the driver expects? E.g. the framebuffer contains too much too bright pixels, or the backlight control is not set appropriately? What may happen on screen in those cases? > > > If we can reuse the same modelines, then rather than create new > > > modelines just for low-power modes, I'd rather create a client CRTC > > > property specifying the number/percentages of pixels on the CRTC which > > > are lit non-zero. That would give us more wriggle room to change the > > > semantics, as well as redefine 'low power' in terms of > > > monochrome/scaled/non-bright/etc modes. But it does make the > > > switching-between-clients problem even worse than it already is. That would seem better to me too, but I got the impression that rather than non-zero, many dim pixels would be ok in low-power too. So that may require specifying the formula for how exactly to calculate the percentage. Mind, that power consumption need not be linear with luminance, so if power consumption is the primary factor then writing it down as luminance may not be correct. Of course, the calculation should be simple and conservative enough that it can be used with many kinds of hardware. Also, HDR monitors may have a similar issue: a monitor may be limited to certain wattage, which means that either you can display a small and very bright patch, or a large and not that bright patch. It's unclear to me if the same mechanism would be appropriate for both limiting HDR display power under normal conditions and cell-phone display power in low-power conditions. Maybe "low power" should not even be a yes/no property, but a value range, like wattage? I think the problem with switching between KMS clients is something we just have to solve by userspace restoring also unrecognized KMS properties on switch-in always, like I have written before. I see no way around it given the policy that the kernel will not offer any kind of "defaults" property set (which too would need to be explicitly used by userspace, or maybe a cap to stop the kernel from applying it automatically whenever something gains DRM master). Thanks, pq > > Yeah, that would make sense too. Or maybe even add read-only hint that > > says "if you're below 15% non-black we can do low power for your, please > > be nice". > > If the hardware can actually do that autonomously then great, but I'm > guessing the reason we're talking about separate opt-in modes here is > that it can't. > > Cheers, > Daniel pgpeDpSoa7NbU.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] video: fbdev: fix divide error in fbcon_switch
Hi
On 22.10.20 01:57, saeed.mirzamohamm...@oracle.com wrote:
> From: Saeed Mirzamohammadi
>
> This patch fixes the issue due to:
>
> [ 89.572883] divide_error: [#1] SMP KASAN PTI
> [ 89.572897] CPU: 3 PID: 16083 Comm: repro Not tainted
> 5.9.0-rc7.20200930.rc1.allarch-19-g3e32d0d.syzk #5
> [ 89.572902] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 0.5.1 01/01/2011
> [ 89.572934] RIP: 0010:cirrusfb_check_var+0x84/0x1260
BTW, if you run qemu with cirrus, there's also a DRM driver named
cirrus.ko. Might be a better choice than the old fbdev driver. If you
just care about qemu, but not the actual graphics device, take a look at
https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/
Anyway, thanks for your patch.
Best regards
Thomas
>
> The error happens when the pixels value is calculated before performing the
> sanity checks on bits_per_pixel.
> A bits_per_pixel set to zero causes divide by zero error.
>
> This patch moves the calculation after the sanity check.
>
> Signed-off-by: Saeed Mirzamohammadi
> Tested-by: Saeed Mirzamohammadi
> ---
> drivers/video/fbdev/cirrusfb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
> index 15a9ee7cd734..a7749101b094 100644
> --- a/drivers/video/fbdev/cirrusfb.c
> +++ b/drivers/video/fbdev/cirrusfb.c
> @@ -531,7 +531,7 @@ static int cirrusfb_check_var(struct fb_var_screeninfo
> *var,
> {
> int yres;
> /* memory size in pixels */
> - unsigned pixels = info->screen_size * 8 / var->bits_per_pixel;
> + unsigned int pixels;
> struct cirrusfb_info *cinfo = info->par;
>
> switch (var->bits_per_pixel) {
> @@ -573,6 +573,7 @@ static int cirrusfb_check_var(struct fb_var_screeninfo
> *var,
> return -EINVAL;
> }
>
> + pixels = info->screen_size * 8 / var->bits_per_pixel;
> if (var->xres_virtual < var->xres)
> var->xres_virtual = var->xres;
> /* use highest possible virtual resolution */
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] video: fbdev: fix divide error in fbcon_switch
Hi
On 22.10.20 01:57, saeed.mirzamohamm...@oracle.com wrote:
> From: Saeed Mirzamohammadi
>
> This patch fixes the issue due to:
>
> [ 89.572883] divide_error: [#1] SMP KASAN PTI
> [ 89.572897] CPU: 3 PID: 16083 Comm: repro Not tainted
> 5.9.0-rc7.20200930.rc1.allarch-19-g3e32d0d.syzk #5
> [ 89.572902] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 0.5.1 01/01/2011
> [ 89.572934] RIP: 0010:cirrusfb_check_var+0x84/0x1260
>
> The error happens when the pixels value is calculated before performing the
> sanity checks on bits_per_pixel.
> A bits_per_pixel set to zero causes divide by zero error.
>
> This patch moves the calculation after the sanity check.
>
> Signed-off-by: Saeed Mirzamohammadi
> Tested-by: Saeed Mirzamohammadi
Looks good, thanks a lot. I'll add the patch to drm-misc-next
Reviewed-by: Thomas Zimemrmann
Best regards
Thomas
> ---
> drivers/video/fbdev/cirrusfb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
> index 15a9ee7cd734..a7749101b094 100644
> --- a/drivers/video/fbdev/cirrusfb.c
> +++ b/drivers/video/fbdev/cirrusfb.c
> @@ -531,7 +531,7 @@ static int cirrusfb_check_var(struct fb_var_screeninfo
> *var,
> {
> int yres;
> /* memory size in pixels */
> - unsigned pixels = info->screen_size * 8 / var->bits_per_pixel;
> + unsigned int pixels;
> struct cirrusfb_info *cinfo = info->par;
>
> switch (var->bits_per_pixel) {
> @@ -573,6 +573,7 @@ static int cirrusfb_check_var(struct fb_var_screeninfo
> *var,
> return -EINVAL;
> }
>
> + pixels = info->screen_size * 8 / var->bits_per_pixel;
> if (var->xres_virtual < var->xres)
> var->xres_virtual = var->xres;
> /* use highest possible virtual resolution */
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 12/16] PCI: Obey iomem restrictions for procfs mmap
On Wed, Oct 21, 2020 at 09:24:08PM +0200, Daniel Vetter wrote: > On Wed, Oct 21, 2020 at 6:37 PM Jason Gunthorpe wrote: > > > > On Wed, Oct 21, 2020 at 05:54:54PM +0200, Daniel Vetter wrote: > > > > > The trouble is that io_remap_pfn adjust vma->pgoff, so we'd need to > > > split that. So ideally ->mmap would never set up any ptes. > > > > /dev/mem makes pgoff == pfn so it doesn't get changed by remap. > > > > pgoff doesn't get touched for MAP_SHARED either, so there are other > > users that could work like this - eg anyone mmaping IO memory is > > probably OK. > > I was more generally thinking for io_remap_pfn_users because of the > mkwrite use-case we might have in fbdev emulation in drm. You have a use case for MAP_PRIVATE and io_remap_pfn_range()?? Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] backlight: pwm_bl: Fix interpolation
On Wed, 14 Oct 2020 at 23:55, Geert Uytterhoeven wrote:
>
> Hi Alexandru,
>
> On Tue, Oct 13, 2020 at 1:57 PM Alexandru Stan wrote:
> > Whenever num-interpolated-steps was larger than the distance
> > between 2 consecutive brightness levels the table would get really
> > discontinuous. The slope of the interpolation would stick with
> > integers only and if it was 0 the whole line segment would get skipped.
> >
> > Example settings:
> > brightness-levels = <0 1 2 4 8 16 32 64 128 256>;
> > num-interpolated-steps = <16>;
> >
> > The distances between 1 2 4 and 8 would be 1, and only starting with 16
> > it would start to interpolate properly.
> >
> > Let's change it so there's always interpolation happening, even if
> > there's no enough points available (read: values in the table would
> > appear more than once). This should match the expected behavior much
> > more closely.
> >
> > Signed-off-by: Alexandru Stan
>
> Thanks for your patch!
Thanks for your reply!
I'm sorry I haven't replied earlier. Looks like your reply was marked as spam.
Rest be assured my spam filter has been disciplined! :D
>
> > --- a/drivers/video/backlight/pwm_bl.c
> > +++ b/drivers/video/backlight/pwm_bl.c
> > @@ -327,24 +324,25 @@ static int pwm_backlight_parse_dt(struct device *dev,
> > table = devm_kzalloc(dev, size, GFP_KERNEL);
> > if (!table)
> > return -ENOMEM;
> > -
> > - /* Fill the interpolated table. */
> > - levels_count = 0;
> > - for (i = 0; i < data->max_brightness - 1; i++) {
> > - value = data->levels[i];
> > - n = (data->levels[i + 1] - value) /
> > num_steps;
> > - if (n > 0) {
> > - for (j = 0; j < num_steps; j++) {
> > - table[levels_count] = value;
> > - value += n;
> > - levels_count++;
> > - }
> > - } else {
> > - table[levels_count] =
> > data->levels[i];
> > - levels_count++;
> > + /*
> > +* Fill the interpolated table[x] = y
> > +* by draw lines between each (x1, y1) to (x2, y2).
> > +*/
> > + dx = num_steps;
> > + for (i = 0; i < num_input_levels - 1; i++) {
> > + x1 = i * dx;
> > + x2 = x1 + dx;
> > + y1 = data->levels[i];
> > + y2 = data->levels[i + 1];
> > + dy = (s64)y2 - y1;
> > +
> > + for (x = x1; x < x2; x++) {
> > + table[x] = y1 +
> > + div_s64(dy * ((s64)x - x1),
> > dx);
>
> Yummy, 64-by-32 divisions.
> Shouldn't this use a rounded division?
It won't hurt. But it really doesn't make much of a difference either way.
>
> Nevertheless, I think it would be worthwhile to implement this using
> a (modified) Bresenham algorithm, avoiding multiplications and
> divisions, and possibly increasing accuracy as well.
>
> https://en.wikipedia.org/wiki/Bresenham%27s_line_algorithm
Sure, it might be a little faster to use Bresenham's line algorithm.
Looks like to implement it I would have to deal with some fixed point
math and still have to do divisions occasionally.
I don't think performance is critical here, the values get calculated
only once when the driver loads, and the algorithm's accuracy
improvements might be at most 1 LSB.
Meanwhile the formula I already implemented is almost the same as the
formulas found at
https://en.wikipedia.org/wiki/Linear_interpolation#:~:text=gives
I would like to keep it as is, as straightforward as possible.
>
> > }
> > }
> > - table[levels_count] = data->levels[i];
> > + /* Fill in the last point, since no line starts
> > here. */
> > + table[x2] = y2;
> >
> > /*
> > * As we use interpolation lets remove current
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
Alexandru Stan (amstan)
Re: [PATCH v3 08/16] s390/pci: Remove races against pte updates
On 10/21/20 10:56 AM, Daniel Vetter wrote:
> Way back it was a reasonable assumptions that iomem mappings never
> change the pfn range they point at. But this has changed:
>
> - gpu drivers dynamically manage their memory nowadays, invalidating
> ptes with unmap_mapping_range when buffers get moved
>
> - contiguous dma allocations have moved from dedicated carvetouts to
> cma regions. This means if we miss the unmap the pfn might contain
> pagecache or anon memory (well anything allocated with GFP_MOVEABLE)
>
> - even /dev/mem now invalidates mappings when the kernel requests that
> iomem region when CONFIG_IO_STRICT_DEVMEM is set, see commit
> 3234ac664a87 ("/dev/mem: Revoke mappings when a driver claims the
> region")
>
> Accessing pfns obtained from ptes without holding all the locks is
> therefore no longer a good idea. Fix this.
>
> Since zpci_memcpy_from|toio seems to not do anything nefarious with
> locks we just need to open code get_pfn and follow_pfn and make sure
> we drop the locks only after we're done. The write function also needs
> the copy_from_user move, since we can't take userspace faults while
> holding the mmap sem.
>
> v2: Move VM_IO | VM_PFNMAP checks around so they keep returning EINVAL
> like before (Gerard)
>
> v3: Polish commit message (Niklas)
>
> Reviewed-by: Gerald Schaefer
> Signed-off-by: Daniel Vetter
> Cc: Jason Gunthorpe
> Cc: Dan Williams
> Cc: Kees Cook
> Cc: Andrew Morton
> Cc: John Hubbard
> Cc: Jérôme Glisse
> Cc: Jan Kara
> Cc: linux...@kvack.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: Niklas Schnelle
> Cc: Gerald Schaefer
> Cc: linux-s...@vger.kernel.org
> Cc: Niklas Schnelle
> Signed-off-by: Daniel Vetter
This should be ".ch", but since this is clearly a typo and you also send from
@ffwll.ch,
I took the liberty and fixed it for this commit and applied your patch to our
internal
branch, Heiko or Vasily will then pick it up for the s390 tree.
Thanks!
> ---
> arch/s390/pci/pci_mmio.c | 98 +++-
> 1 file changed, 57 insertions(+), 41 deletions(-)
>
> diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
> index 401cf670a243..1a6adbc68ee8 100644
> --- a/arch/s390/pci/pci_mmio.c
> +++ b/arch/s390/pci/pci_mmio.c
> @@ -119,33 +119,15 @@ static inline int __memcpy_toio_inuser(void __iomem
> *dst,
> return rc;
> }
>
> -static long get_pfn(unsigned long user_addr, unsigned long access,
> - unsigned long *pfn)
> -{
> - struct vm_area_struct *vma;
> - long ret;
> -
> - mmap_read_lock(current->mm);
> - ret = -EINVAL;
> - vma = find_vma(current->mm, user_addr);
> - if (!vma)
> - goto out;
> - ret = -EACCES;
> - if (!(vma->vm_flags & access))
> - goto out;
> - ret = follow_pfn(vma, user_addr, pfn);
> -out:
> - mmap_read_unlock(current->mm);
> - return ret;
> -}
> -
> SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
> const void __user *, user_buffer, size_t, length)
> {
> u8 local_buf[64];
> void __iomem *io_addr;
> void *buf;
> - unsigned long pfn;
> + struct vm_area_struct *vma;
> + pte_t *ptep;
> + spinlock_t *ptl;
> long ret;
>
> if (!zpci_is_enabled())
> @@ -158,7 +140,7 @@ SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long,
> mmio_addr,
>* We only support write access to MIO capable devices if we are on
>* a MIO enabled system. Otherwise we would have to check for every
>* address if it is a special ZPCI_ADDR and would have to do
> - * a get_pfn() which we don't need for MIO capable devices. Currently
> + * a pfn lookup which we don't need for MIO capable devices. Currently
>* ISM devices are the only devices without MIO support and there is no
>* known need for accessing these from userspace.
>*/
> @@ -176,21 +158,37 @@ SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long,
> mmio_addr,
> } else
> buf = local_buf;
>
> - ret = get_pfn(mmio_addr, VM_WRITE, );
> + ret = -EFAULT;
> + if (copy_from_user(buf, user_buffer, length))
> + goto out_free;
> +
> + mmap_read_lock(current->mm);
> + ret = -EINVAL;
> + vma = find_vma(current->mm, mmio_addr);
> + if (!vma)
> + goto out_unlock_mmap;
> + if (!(vma->vm_flags & (VM_IO | VM_PFNMAP)))
> + goto out_unlock_mmap;
> + ret = -EACCES;
> + if (!(vma->vm_flags & VM_WRITE))
> + goto out_unlock_mmap;
> +
> + ret = follow_pte_pmd(vma->vm_mm, mmio_addr, NULL, , NULL, );
> if (ret)
> - goto out;
> - io_addr = (void __iomem *)((pfn << PAGE_SHIFT) |
> + goto out_unlock_mmap;
> +
> + io_addr = (void __iomem *)((pte_pfn(*ptep) << PAGE_SHIFT) |
>
Re: [PATCH v2 08/17] s390/pci: Remove races against pte updates
Hi Daniel,
friendly ping. I haven't seen a new version of this patch series,
as I said I think your change for s390/pci is generally useful so
I'm curious, are you planning on sending a new version soon?
If you want you can also just sent this patch with the last few
nitpicks (primarily the mail address) fixed and I'll happily apply.
Best regards,
Niklas Schnelle
On 10/12/20 4:19 PM, Daniel Vetter wrote:
> On Mon, Oct 12, 2020 at 04:03:28PM +0200, Niklas Schnelle wrote:
... snip
>>> Cc: Jason Gunthorpe
>>> Cc: Dan Williams
>>> Cc: Kees Cook
>>> Cc: Andrew Morton
>>> Cc: John Hubbard
>>> Cc: Jérôme Glisse
>>> Cc: Jan Kara
>>> Cc: Dan Williams
>>
>> The above Cc: line for Dan Williams is a duplicate
>>
>>> Cc: linux...@kvack.org
>>> Cc: linux-arm-ker...@lists.infradead.org
>>> Cc: linux-samsung-...@vger.kernel.org
>>> Cc: linux-me...@vger.kernel.org
>>> Cc: Niklas Schnelle
>>> Cc: Gerald Schaefer
>>> Cc: linux-s...@vger.kernel.org
>>> --
>>> v2: Move VM_IO | VM_PFNMAP checks around so they keep returning EINVAL
>>> like before (Gerard)
>>
>> I think the above should go before the CC/Signed-off/Reviewev block.
>
> This is a per-subsystem bikeshed :-) drivers/gpu definitely wants it
> above, but most core subsystems want it below. I'll move it.
>
>>> ---
>>> arch/s390/pci/pci_mmio.c | 98 +++-
>>> 1 file changed, 57 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
>>> index 401cf670a243..1a6adbc68ee8 100644
>>> --- a/arch/s390/pci/pci_mmio.c
>>> +++ b/arch/s390/pci/pci_mmio.c
>>> @@ -119,33 +119,15 @@ static inline int __memcpy_toio_inuser(void __iomem
>>> *dst,
>>> return rc;
>>> }
>>>
>>> -static long get_pfn(unsigned long user_addr, unsigned long access,
>>> - unsigned long *pfn)
>>> -{
>>> - struct vm_area_struct *vma;
>>> - long ret;
>>> -
>>> - mmap_read_lock(current->mm);
>>> - ret = -EINVAL;
>>> - vma = find_vma(current->mm, user_addr);
>>> - if (!vma)
>>> - goto out;
>>> - ret = -EACCES;
>>> - if (!(vma->vm_flags & access))
>>> - goto out;
>>> - ret = follow_pfn(vma, user_addr, pfn);
>>> -out:
>>> - mmap_read_unlock(current->mm);
>>> - return ret;
>>> -}
>>> -
>>> SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
>>> const void __user *, user_buffer, size_t, length)
>>> {
>>> u8 local_buf[64];
>>> void __iomem *io_addr;
>>> void *buf;
>>> - unsigned long pfn;
>>> + struct vm_area_struct *vma;
>>> + pte_t *ptep;
>>> + spinlock_t *ptl;
>>
>> With checkpatch.pl --strict the above yields a complained
>> "CHECK: spinlock_t definition without comment" but I think
>> that's really okay since your commit description is very clear.
>> Same oin line 277.
>
> I think this is a falls positive, checkpatch doesn't realize that
> SYSCALL_DEFINE3 is a function, not a structure. And in a structure I'd
> have added the kerneldoc or comment.
>
> I'll fix up all the nits you've found for the next round. Thanks for
> taking a look.
> -Daniel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 12/16] PCI: Obey iomem restrictions for procfs mmap
On Wed, Oct 21, 2020 at 04:42:11PM +0200, Daniel Vetter wrote: > Uh yes. In drivers/gpu this isn't a problem because we only install > ptes from the vm_ops->fault handler. So no races. And I don't think > you can fix this otherwise through holding locks: mmap_sem we can't > hold because before vma_link we don't even know which mm_struct is > involved, so can't solve the race. Plus this would be worse that > mm_take_all_locks used by mmu notifier. And the address_space > i_mmap_lock is also no good since it's not held during the ->mmap > callback, when we write the ptes. And the resource locks is even less > useful, since we're not going to hold that at vma_link() time for > sure. > > Hence delaying the pte writes after the vma_link, which means ->fault > time, looks like the only way to close this gap. > Trouble is I have no idea how to do this cleanly ... How about add a vm_ops callback 'install_pages'/'prefault_pages' ? Call it after vm_link() - basically just move the remap_pfn, under some other lock, into there. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/3] PWM backlight interpolation adjustments
I was trying to adjust the brightness-levels for the trogdor boards: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2291209 Like on a lot of panels, trogdor's low end needs to be cropped, and now that we have the interpolation stuff I wanted to make use of it and bake in even the curve that's customary to have on chromebooks. I found the current behavior of the pwm_bl driver a little unintuitive and non-linear. See patch 1 for a suggested fix for this. A few veyron dts files were relying on this (perhaps weird) behavior. Those devices also want a minimum brightness like trogdor, so changed them to use the new way. Finally, given that trogdor's dts is part of linux-next now, add the brightness-levels to it, since that's the original reason I was looking at this. Changes in v3: - Reordered patches, since both dts changes will work just fine even before the driver change. - Rewrote a bit of the commit message to describe the new policy, as Daniel suggested. - Removed redundant s64 for something that's always positive Changes in v2: - Fixed type promotion in the driver - Removed "backlight: pwm_bl: Artificially add 0% during interpolation", userspace works just fine without it because it already knows how to use bl_power for turning off the display. - Added brightness-levels to trogdor as well, now the dts is upstream. Alexandru Stan (3): ARM: dts: rockchip: veyron: Remove 0 point from brightness-levels arm64: dts: qcom: trogdor: Add brightness-levels backlight: pwm_bl: Fix interpolation arch/arm/boot/dts/rk3288-veyron-jaq.dts | 2 +- arch/arm/boot/dts/rk3288-veyron-minnie.dts | 2 +- arch/arm/boot/dts/rk3288-veyron-tiger.dts| 2 +- arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 9 +++ drivers/video/backlight/pwm_bl.c | 70 +--- 5 files changed, 43 insertions(+), 42 deletions(-) -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2 3/8] drm/msm: Unconditionally call dev_pm_opp_of_remove_table()
On 05-10-20, 11:56, Viresh Kumar wrote: > On 28-08-20, 11:37, Viresh Kumar wrote: > > dev_pm_opp_of_remove_table() doesn't report any errors when it fails to > > find the OPP table with error -ENODEV (i.e. OPP table not present for > > the device). And we can call dev_pm_opp_of_remove_table() > > unconditionally here. > > > > While at it, also create a label to put clkname. > > > > Signed-off-by: Viresh Kumar > > Can someone please apply this and the other drm patch (2/8) ? Rob/Rajendra, can someone please have a look at these patches ? -- viresh ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 00/16] follow_pfn and other iomap races
On Wed, Oct 21, 2020 at 10:56:39AM +0200, Daniel Vetter wrote: > Hi all, > > Round 3 of my patch series to clamp down a bunch of races and gaps > around follow_pfn and other access to iomem mmaps. Previous version: > > v1: > https://lore.kernel.org/dri-devel/20201007164426.1812530-1-daniel.vet...@ffwll.ch/ > v2: > https://lore.kernel.org/dri-devel/20201009075934.3509076-1-daniel.vet...@ffwll.ch > > And the discussion that sparked this journey: > > https://lore.kernel.org/dri-devel/20201007164426.1812530-1-daniel.vet...@ffwll.ch/ > > I was waiting for the testing result for habanalabs from Oded, but I guess > Oded was waiting for my v3. > > Changes in v3: > - Bunch of polish all over, no functional changes aside from one barrier > in the resource code, for consistency. > - A few more r-b tags. > > Changes in v2: > - tons of small polish all over, thanks to all the reviewers who > spotted issues > - I managed to test at least the generic_access_phys and pci mmap revoke > stuff with a few gdb sessions using our i915 debug tools (hence now also > the drm/i915 patch to properly request all the pci bar regions) > - reworked approach for the pci mmap revoke: Infrastructure moved into > kernel/resource.c, address_space mapping is now set up at open time for > everyone (which required some sysfs changes). Does indeed look a lot > cleaner and a lot less invasive than I feared at first. > > The big thing I can't test are all the frame_vector changes in habanalbas, > exynos and media. Gerald has given the s390 patch a spin already. > > Review, testing, feedback all very much welcome. > > Daniel Vetter (16): > drm/exynos: Stop using frame_vector helpers > drm/exynos: Use FOLL_LONGTERM for g2d cmdlists > misc/habana: Stop using frame_vector helpers > misc/habana: Use FOLL_LONGTERM for userptr > mm/frame-vector: Use FOLL_LONGTERM > media: videobuf2: Move frame_vector into media subsystem > mm: Close race in generic_access_phys > s390/pci: Remove races against pte updates > mm: Add unsafe_follow_pfn > media/videbuf1|2: Mark follow_pfn usage as unsafe > vfio/type1: Mark follow_pfn as unsafe > PCI: Obey iomem restrictions for procfs mmap > /dev/mem: Only set filp->f_mapping > resource: Move devmem revoke code to resource framework > sysfs: Support zapping of binary attr mmaps > PCI: Revoke mappings like devmem The whole thing looks like a great improvement! Thanks, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 12/16] PCI: Obey iomem restrictions for procfs mmap
On Wed, Oct 21, 2020 at 10:56:51AM +0200, Daniel Vetter wrote:
> There's three ways to access PCI BARs from userspace: /dev/mem, sysfs
> files, and the old proc interface. Two check against
> iomem_is_exclusive, proc never did. And with CONFIG_IO_STRICT_DEVMEM,
> this starts to matter, since we don't want random userspace having
> access to PCI BARs while a driver is loaded and using it.
>
> Fix this by adding the same iomem_is_exclusive() check we already have
> on the sysfs side in pci_mmap_resource().
>
> References: 90a545e98126 ("restrict /dev/mem to idle io memory ranges")
> Signed-off-by: Daniel Vetter
> Cc: Jason Gunthorpe
> Cc: Kees Cook
> Cc: Dan Williams
> Cc: Andrew Morton
> Cc: John Hubbard
> Cc: Jérôme Glisse
> Cc: Jan Kara
> Cc: Dan Williams
> Cc: linux...@kvack.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: Bjorn Helgaas
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Daniel Vetter
Maybe not for fixing in this series, but this access to
IORESOURCE_BUSY doesn't have any locking.
The write side holds the resource_lock at least..
> ret = pci_mmap_page_range(dev, i, vma,
> fpriv->mmap_state, write_combine);
At this point the vma isn't linked into the address space, so doesn't
this happen?
CPU 0 CPU1
mmap_region()
vma = vm_area_alloc
proc_bus_pci_mmap
iomem_is_exclusive
pci_mmap_page_range
revoke_devmem
unmap_mapping_range()
// vma is not linked to the address space here,
// unmap doesn't find it
vma_link()
!!! The VMA gets mapped with the revoked PTEs
I couldn't find anything that prevents it at least, no mmap_sem on the
unmap side, just the i_mmap_lock
Not seeing how address space and pre-populating during mmap work
together? Did I miss locking someplace?
Not something to be fixed for this series, this is clearly an
improvement, but seems like another problem to tackle?
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2 3/8] drm/msm: Unconditionally call dev_pm_opp_of_remove_table()
On 21-10-20, 09:58, Rob Clark wrote: > On Wed, Oct 21, 2020 at 12:24 AM Viresh Kumar wrote: > > > > On 05-10-20, 11:56, Viresh Kumar wrote: > > > On 28-08-20, 11:37, Viresh Kumar wrote: > > > > dev_pm_opp_of_remove_table() doesn't report any errors when it fails to > > > > find the OPP table with error -ENODEV (i.e. OPP table not present for > > > > the device). And we can call dev_pm_opp_of_remove_table() > > > > unconditionally here. > > > > > > > > While at it, also create a label to put clkname. > > > > > > > > Signed-off-by: Viresh Kumar > > > > > > Can someone please apply this and the other drm patch (2/8) ? > > > > Rob/Rajendra, can someone please have a look at these patches ? > > Oh, sorry I missed that, could someone re-send it and CC > freedr...@lists.freedesktop.org so it shows up in patchworks.. that is > more reliable counting on me to not overlook something in my mail I have just bounced it back to you and freedreno was already cc'd, though I have bounced it there again. -- viresh ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/sun4i: frontend: Reuse the ch0 phase for RGB formats
Hi! Dne četrtek, 15. oktober 2020 ob 11:36:41 CEST je Maxime Ripard napisal(a): > When using the scaler on the A10-like frontend with single-planar formats, > the current code will setup the channel 0 filter (used for the R or Y > component) with a different phase parameter than the channel 1 filter (used > for the G/B or U/V components). > > This creates a bleed out that keeps repeating on of the last line of the > RGB plane across the rest of the display. The Allwinner BSP either applies > the same phase parameter over both channels or use a separate one, the > condition being whether the input format is YUV420 or not. > > Since YUV420 is both subsampled and multi-planar, and since YUYV is > subsampled but single-planar, we can rule out the subsampling and assume > that the condition is actually whether the format is single or > multi-planar. And it looks like applying the same phase parameter over both > channels for single-planar formats fixes our issue, while we keep the > multi-planar formats working properly. > > Reported-by: Taras Galchenko > Signed-off-by: Maxime Ripard Acked-by: Jernej Skrabec Best regards, Jernej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2 2/8] drm/lima: Unconditionally call dev_pm_opp_of_remove_table()
On 28-08-20, 11:37, Viresh Kumar wrote: > dev_pm_opp_of_remove_table() doesn't report any errors when it fails to > find the OPP table with error -ENODEV (i.e. OPP table not present for > the device). And we can call dev_pm_opp_of_remove_table() > unconditionally here. > > Reviewed-by: Qiang Yu > Signed-off-by: Viresh Kumar > > --- > V2: Applied Reviewed by tag. > --- > drivers/gpu/drm/lima/lima_devfreq.c | 6 +- > drivers/gpu/drm/lima/lima_devfreq.h | 1 - > 2 files changed, 1 insertion(+), 6 deletions(-) Qiang, can you please pick it up ? -- viresh ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/1] video: fbdev: fix divide error in fbcon_switch
From: Saeed Mirzamohammadi
This patch fixes the issue due to:
[ 89.572883] divide_error: [#1] SMP KASAN PTI
[ 89.572897] CPU: 3 PID: 16083 Comm: repro Not tainted
5.9.0-rc7.20200930.rc1.allarch-19-g3e32d0d.syzk #5
[ 89.572902] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
0.5.1 01/01/2011
[ 89.572934] RIP: 0010:cirrusfb_check_var+0x84/0x1260
The error happens when the pixels value is calculated before performing the
sanity checks on bits_per_pixel.
A bits_per_pixel set to zero causes divide by zero error.
This patch moves the calculation after the sanity check.
Signed-off-by: Saeed Mirzamohammadi
Tested-by: Saeed Mirzamohammadi
---
drivers/video/fbdev/cirrusfb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
index 15a9ee7cd734..a7749101b094 100644
--- a/drivers/video/fbdev/cirrusfb.c
+++ b/drivers/video/fbdev/cirrusfb.c
@@ -531,7 +531,7 @@ static int cirrusfb_check_var(struct fb_var_screeninfo *var,
{
int yres;
/* memory size in pixels */
- unsigned pixels = info->screen_size * 8 / var->bits_per_pixel;
+ unsigned int pixels;
struct cirrusfb_info *cinfo = info->par;
switch (var->bits_per_pixel) {
@@ -573,6 +573,7 @@ static int cirrusfb_check_var(struct fb_var_screeninfo *var,
return -EINVAL;
}
+ pixels = info->screen_size * 8 / var->bits_per_pixel;
if (var->xres_virtual < var->xres)
var->xres_virtual = var->xres;
/* use highest possible virtual resolution */
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/sun4i: frontend: Rework a bit the phase data
Hi! Dne četrtek, 15. oktober 2020 ob 11:36:40 CEST je Maxime Ripard napisal(a): > The scaler filter phase setup in the allwinner kernel has two different > cases for setting up the scaler filter, the first one using different phase > parameters for the two channels, and the second one reusing the first > channel parameters on the second channel. > > The allwinner kernel has a third option where the horizontal phase of the > second channel will be set to a different value than the vertical one (and > seems like it's the same value than one used on the first channel). > However, that code path seems to never be taken, so we can ignore it for > now, and it's essentially what we're doing so far as well. > > Since we will have always the same values across each components of the > filter setup for a given channel, we can simplify a bit our frontend > structure by only storing the phase value we want to apply to a given > channel. > > Signed-off-by: Maxime Ripard Acked-by: Jernej Skrabec Best regards, Jernej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
drm_modes: signed integer overflow
Hi, With linux-next 20201021, when booting up, I am seeing this: [0.560896] UBSAN: signed-integer-overflow in ../drivers/gpu/drm/drm_modes.c:765:20 [0.560903] 2376000 * 1000 cannot be represented in type 'int' [0.560909] CPU: 3 PID: 7 Comm: kworker/u16:0 Not tainted 5.9.0-next-20201021 #2 [0.560914] Hardware name: TOSHIBA PORTEGE R835/Portable PC, BIOS Version 4.10 01/08/2013 [0.560924] Workqueue: events_unbound async_run_entry_fn [0.560930] Call Trace: [0.560938] dump_stack+0x5e/0x74 [0.560943] ubsan_epilogue+0x9/0x45 [0.560948] handle_overflow+0x8b/0x98 [0.560953] ? set_track+0x3f/0xad [0.560958] __ubsan_handle_mul_overflow+0xe/0x10 [0.560964] drm_mode_vrefresh+0x4a/0xbc [0.560970] initcall i915_init+0x0/0x6a returned 0 after 116076 usecs [0.560974] calling cn_proc_init+0x0/0x36 @ 1 [0.560978] cea_mode_alternate_clock+0x11/0x62 [0.560985] drm_match_cea_mode+0xc7/0x1e7 [0.560987] initcall cn_proc_init+0x0/0x36 returned 0 after 3 usecs [0.560990] calling topology_sysfs_init+0x0/0x2d @ 1 [0.561000] drm_mode_validate_ycbcr420+0xd/0x48 [0.561005] drm_helper_probe_single_connector_modes+0x6db/0x7da [0.561012] drm_client_modeset_probe+0x225/0x143f [0.561018] ? bitmap_fold+0x8a/0x8a [0.561023] ? update_cfs_rq_load_avg+0x192/0x1a2 [0.561029] __drm_fb_helper_initial_config_and_unlock+0x3f/0x5b7 [0.561035] ? get_sd_balance_interval+0x1c/0x40 [0.561040] drm_fb_helper_initial_config+0x48/0x4f [0.561047] intel_fbdev_initial_config+0x13/0x23 [0.561052] async_run_entry_fn+0x89/0x15c [0.561058] process_one_work+0x15c/0x1f3 [0.561064] worker_thread+0x1ac/0x25d [0.561069] ? process_scheduled_works+0x2e/0x2e [0.561074] kthread+0x10e/0x116 [0.561078] ? kthread_parkme+0x1c/0x1c [0.561083] ret_from_fork+0x22/0x30 [0.561087] -- ~Randy Reported-by: Randy Dunlap ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RESEND v2] drm/bridge/tc358775: Fixes bus formats read
- atomic_check removed
- video data input and output formats added
- bus formats read from drm_bridge_state.output_bus_cfg.format
and .atomic_get_input_bus_fmts() instead of connector
Signed-off-by: Vinay Simha BN
---
v1:
* Laurent Pinchart review comments incorporated
drm_bridge_state.output_bus_cfg.format
instead of connector
v2:
* Laurent Pinchart review comments incorporated
atomic_check removed
video data input and output formats added
---
drivers/gpu/drm/bridge/tc358775.c | 75 ++-
1 file changed, 58 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 2272adc..cc27570 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -271,6 +271,20 @@ struct tc_data {
struct gpio_desc*stby_gpio;
u8 lvds_link; /* single-link or dual-link */
u8 bpc;
+ u32 output_bus_fmt;
+};
+
+static const u32 tc_lvds_in_bus_fmts[] = {
+ MEDIA_BUS_FMT_RGB565_1X16,
+ MEDIA_BUS_FMT_RGB666_1X18,
+ MEDIA_BUS_FMT_RGB666_1X24_CPADHI,
+ MEDIA_BUS_FMT_RBG888_1X24,
+};
+
+static const u32 tc_lvds_out_bus_fmts[] = {
+ MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA,
+ MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
+ MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
};
static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
@@ -359,19 +373,6 @@ static void d2l_write(struct i2c_client *i2c, u16 addr,
u32 val)
ret, addr);
}
-/* helper function to access bus_formats */
-static struct drm_connector *get_connector(struct drm_encoder *encoder)
-{
- struct drm_device *dev = encoder->dev;
- struct drm_connector *connector;
-
- list_for_each_entry(connector, >mode_config.connector_list, head)
- if (connector->encoder == encoder)
- return connector;
-
- return NULL;
-}
-
static void tc_bridge_enable(struct drm_bridge *bridge)
{
struct tc_data *tc = bridge_to_tc(bridge);
@@ -380,7 +381,10 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
u32 val = 0;
u16 dsiclk, clkdiv, byteclk, t1, t2, t3, vsdelay;
struct drm_display_mode *mode;
- struct drm_connector *connector = get_connector(bridge->encoder);
+ struct drm_bridge_state *state =
+ drm_priv_to_bridge_state(bridge->base.state);
+
+ tc->output_bus_fmt = state->output_bus_cfg.format;
mode = >encoder->crtc->state->adjusted_mode;
@@ -451,14 +455,13 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
d2l_write(tc->i2c, LVPHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_ND(6));
dev_dbg(tc->dev, "bus_formats %04x bpc %d\n",
- connector->display_info.bus_formats[0],
+ tc->output_bus_fmt,
tc->bpc);
/*
* Default hardware register settings of tc358775 configured
* with MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA jeida-24 format
*/
- if (connector->display_info.bus_formats[0] ==
- MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
+ if (tc->output_bus_fmt == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
/* VESA-24 */
d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2,
LVI_R3));
d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5,
LVI_G0));
@@ -590,6 +593,40 @@ static int tc358775_parse_dt(struct device_node *np,
struct tc_data *tc)
return 0;
}
+static u32 *
+tc_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
+struct drm_bridge_state *bridge_state,
+struct drm_crtc_state *crtc_state,
+struct drm_connector_state *conn_state,
+u32 output_fmt,
+unsigned int *num_input_fmts)
+{
+ u32 *input_fmts = NULL;
+ u8 i;
+
+ *num_input_fmts = 0;
+
+ for (i = 0 ; i < ARRAY_SIZE(tc_lvds_out_bus_fmts) ; ++i) {
+ if (output_fmt == tc_lvds_out_bus_fmts[i])
+ break;
+ }
+
+ if (i == ARRAY_SIZE(tc_lvds_out_bus_fmts))
+ return NULL;
+
+ *num_input_fmts = ARRAY_SIZE(tc_lvds_in_bus_fmts);
+
+ input_fmts = kcalloc(*num_input_fmts, ARRAY_SIZE(tc_lvds_in_bus_fmts),
+GFP_KERNEL);
+ if (!input_fmts)
+ return NULL;
+
+ for (i = 0; i < ARRAY_SIZE(tc_lvds_in_bus_fmts); ++i)
+ input_fmts[i] = tc_lvds_in_bus_fmts[i];
+
+ return input_fmts;
+}
+
static int tc_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)
{
@@ -639,6 +676,10 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
}
static const struct drm_bridge_funcs tc_bridge_funcs = {
+ .atomic_duplicate_state =
[Outreachy kernel][PATCH] gpu: amd: Return boolean types instead of integer values
Return statements for functions returning bool should use truth and false instead of 1 and 0 respectively. Modify cik_event_interrupt.c to return false instead of 0. Issue found with Coccinelle. Signed-off-by: Sumera Priyadarsini --- drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c index 24b471734117..8e64c01565ac 100644 --- a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c +++ b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c @@ -66,12 +66,12 @@ static bool cik_event_interrupt_isr(struct kfd_dev *dev, vmid = (ihre->ring_id & 0xff00) >> 8; if (vmid < dev->vm_info.first_vmid_kfd || vmid > dev->vm_info.last_vmid_kfd) - return 0; + return false; /* If there is no valid PASID, it's likely a firmware bug */ pasid = (ihre->ring_id & 0x) >> 16; if (WARN_ONCE(pasid == 0, "FW bug: No PASID in KFD interrupt")) - return 0; + return false; /* Interrupt types we care about: various signals and faults. * They will be forwarded to a work queue (see below). -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING in dma_map_page_attrs
Hello, syzbot found the following issue on: HEAD commit:c4d6fe73 Merge tag 'xarray-5.9' of git://git.infradead.org.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=14862ff050 kernel config: https://syzkaller.appspot.com/x/.config?x=7d790573d3e379c4 dashboard link: https://syzkaller.appspot.com/bug?extid=34dc2fea3478e659af01 compiler: gcc (GCC) 10.1.0-syz 20200507 Unfortunately, I don't have any reproducer for this issue yet. IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+34dc2fea3478e659a...@syzkaller.appspotmail.com infiniband syz1: set active infiniband syz1: added vcan0 [ cut here ] WARNING: CPU: 1 PID: 9851 at kernel/dma/mapping.c:149 dma_map_page_attrs+0x493/0x700 kernel/dma/mapping.c:149 Modules linked in: CPU: 1 PID: 9851 Comm: syz-executor.1 Not tainted 5.9.0-syzkaller #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 RIP: 0010:dma_map_page_attrs+0x493/0x700 kernel/dma/mapping.c:149 Code: 80 3c 10 00 0f 85 ed 01 00 00 48 8b 1d 36 c3 fa 0c e9 2d fc ff ff 48 89 c3 e9 d1 fd ff ff e8 04 12 12 00 0f 0b e8 fd 11 12 00 <0f> 0b 49 c7 c4 ff ff ff ff e9 d5 fd ff ff e8 ea 11 12 00 48 8d 7b RSP: 0018:c90001546c68 EFLAGS: 00010246 RAX: 0004 RBX: 894d0040 RCX: c9000dbe4000 RDX: 0004 RSI: 815d3b03 RDI: 88806a988b00 RBP: 8880236cc400 R08: 0002 R09: R10: 0002 R11: R12: ea8db300 R13: 88806a9886e8 R14: 04b8 R15: 0002 FS: 7f678fae2700() GS:88802ce0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f299a39b190 CR3: 69f31000 CR4: 00350ee0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: dma_map_single_attrs include/linux/dma-mapping.h:279 [inline] ib_dma_map_single include/rdma/ib_verbs.h:3967 [inline] ib_mad_post_receive_mads+0x23f/0xd60 drivers/infiniband/core/mad.c:2715 ib_mad_port_start drivers/infiniband/core/mad.c:2862 [inline] ib_mad_port_open drivers/infiniband/core/mad.c:3016 [inline] ib_mad_init_device+0x72b/0x1400 drivers/infiniband/core/mad.c:3092 add_client_context+0x405/0x5e0 drivers/infiniband/core/device.c:680 enable_device_and_get+0x1d5/0x3c0 drivers/infiniband/core/device.c:1301 ib_register_device drivers/infiniband/core/device.c:1376 [inline] ib_register_device+0x7a7/0xa40 drivers/infiniband/core/device.c:1335 rxe_register_device+0x46d/0x570 drivers/infiniband/sw/rxe/rxe_verbs.c:1182 rxe_add+0x12fe/0x16d0 drivers/infiniband/sw/rxe/rxe.c:247 rxe_net_add+0x8c/0xe0 drivers/infiniband/sw/rxe/rxe_net.c:507 rxe_newlink drivers/infiniband/sw/rxe/rxe.c:269 [inline] rxe_newlink+0xb7/0xe0 drivers/infiniband/sw/rxe/rxe.c:250 nldev_newlink+0x30e/0x540 drivers/infiniband/core/nldev.c:1555 rdma_nl_rcv_msg+0x367/0x690 drivers/infiniband/core/netlink.c:195 rdma_nl_rcv_skb drivers/infiniband/core/netlink.c:239 [inline] rdma_nl_rcv+0x2f2/0x440 drivers/infiniband/core/netlink.c:259 netlink_unicast_kernel net/netlink/af_netlink.c:1304 [inline] netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1330 netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1919 sock_sendmsg_nosec net/socket.c:651 [inline] sock_sendmsg+0xcf/0x120 net/socket.c:671 sys_sendmsg+0x6e8/0x810 net/socket.c:2353 ___sys_sendmsg+0xf3/0x170 net/socket.c:2407 __sys_sendmsg+0xe5/0x1b0 net/socket.c:2440 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x45d9f9 Code: bd b1 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 8b b1 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f678fae1c88 EFLAGS: 0246 ORIG_RAX: 002e RAX: ffda RBX: 0071f480 RCX: 0045d9f9 RDX: RSI: 2200 RDI: 0003 RBP: 004aab13 R08: R09: R10: R11: 0246 R12: 0075bf00 R13: 7ffc6f9b8bbf R14: 7f678fac2000 R15: 0003 --- This report is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this issue. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 19/20] drm/tegra: Implement new UAPI
20.10.2020 12:18, Mikko Perttunen пишет: >> I'm asking this question because right now there is only one potential >> client for this IOCTL, the VIC. If other clients aren't supposed to be a >> part of the DRM driver, like for example NVDEC which probably should be >> a V4L driver, then DRM driver will have only a single VIC and in this >> case we shouldn't need this IOCTL because DRM and V4L should use generic >> dmabuf API for importing and exporting buffers. > > This IOCTL is required for GR2D/GR3D too, as they need to access memory > as well. This is a different step from import/export -- first you import > or allocate your memory so you have a GEM handle, then you map it to the > channel, which does the IOMMU mapping (if there is an IOMMU). > This doesn't answer my question. I don't have a full picture and for now will remain dubious about this IOCTL, but it should be fine to have it in a form of WIP patches (maybe staging feature) until userspace code and hardware specs will become available. Some more comments: 1. Older Tegra SoCs do not have restrictions which do not allow to group IOMMU as wished by kernel driver. It's fine to have one static mapping per-GEM that can be accessed by all DRM devices, that's why CHANNEL_MAP is questionable. 2. IIUC, the mappings need to be done per-device group/stream and not per-channel_ctx. It looks like nothing stops channel contexts to guess mapping addresses of the other context, isn't it? I'm suggesting that each GEM should have a per-device mapping and the new IOCTL should create these GEM-mappings, instead of the channel_ctx mappings. 3. We shouldn't need channel contexts at all, a single "DRM file" context should be enough to have. 4. The new UAPI need to be separated into several parts in the next revision, one patch for each new feature. The first patches should be the ones that are relevant to the existing userspace code, like support for the waits. Partial mappings should be a separate feature because it's a questionable feature that needs to be proved by a real userspace first. Maybe it would be even better to drop it for the starter, to ease reviewing. Waiting for fences on host1x should be a separate feature. The syncfile support needs to be a separate feature as well because I don't see a use-case for it right now. I'd like to see the DRM_SCHED and syncobj support. I can help you with it if it's out of yours scope for now. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] video: fbdev: fix divide error in fbcon_switch
Attached the Syzkaller C reproducer.
repro.c
Description: Binary data
> On Oct 21, 2020, at 4:57 PM, saeed.mirzamohamm...@oracle.com wrote:
>
> From: Saeed Mirzamohammadi
>
> This patch fixes the issue due to:
>
> [ 89.572883] divide_error: [#1] SMP KASAN PTI
> [ 89.572897] CPU: 3 PID: 16083 Comm: repro Not tainted
> 5.9.0-rc7.20200930.rc1.allarch-19-g3e32d0d.syzk #5
> [ 89.572902] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 0.5.1 01/01/2011
> [ 89.572934] RIP: 0010:cirrusfb_check_var+0x84/0x1260
>
> The error happens when the pixels value is calculated before performing the
> sanity checks on bits_per_pixel.
> A bits_per_pixel set to zero causes divide by zero error.
>
> This patch moves the calculation after the sanity check.
>
> Signed-off-by: Saeed Mirzamohammadi
> Tested-by: Saeed Mirzamohammadi
> ---
> drivers/video/fbdev/cirrusfb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
> index 15a9ee7cd734..a7749101b094 100644
> --- a/drivers/video/fbdev/cirrusfb.c
> +++ b/drivers/video/fbdev/cirrusfb.c
> @@ -531,7 +531,7 @@ static int cirrusfb_check_var(struct fb_var_screeninfo
> *var,
> {
> int yres;
> /* memory size in pixels */
> - unsigned pixels = info->screen_size * 8 / var->bits_per_pixel;
> + unsigned int pixels;
> struct cirrusfb_info *cinfo = info->par;
>
> switch (var->bits_per_pixel) {
> @@ -573,6 +573,7 @@ static int cirrusfb_check_var(struct fb_var_screeninfo
> *var,
> return -EINVAL;
> }
>
> + pixels = info->screen_size * 8 / var->bits_per_pixel;
> if (var->xres_virtual < var->xres)
> var->xres_virtual = var->xres;
> /* use highest possible virtual resolution */
> --
> 2.27.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 12/16] PCI: Obey iomem restrictions for procfs mmap
On Wed, Oct 21, 2020 at 05:54:54PM +0200, Daniel Vetter wrote: > The trouble is that io_remap_pfn adjust vma->pgoff, so we'd need to > split that. So ideally ->mmap would never set up any ptes. /dev/mem makes pgoff == pfn so it doesn't get changed by remap. pgoff doesn't get touched for MAP_SHARED either, so there are other users that could work like this - eg anyone mmaping IO memory is probably OK. > I guess one option would be if remap_pfn_range would steal the > vma->vm_ops pointer for itself, then it could set up the correct > ->install_ptes hook. But there's tons of callers for that, so not sure > that's a bright idea. The caller has to check that the mapping is still live, and I think hold a lock across the remap? Auto-defering it doesn't seem feasible. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/8] dt-bindings: display: mediatek: dsi: add documentation for MT8167 SoC
Hi Chun-Kuang,
On Wed, Oct 21, 2020 at 7:01 PM Chun-Kuang Hu wrote:
>
> Hi, Fabien:
>
> Fabien Parent 於 2020年10月21日 週三 上午1:43寫道：
> >
> > Add binding documentation for the MT8167 SoC. The SoC needs
> > an additional clock compared to the already supported SoC: mipi26m.
> >
> > Signed-off-by: Fabien Parent
> > ---
> > .../devicetree/bindings/display/mediatek/mediatek,dsi.txt | 7 ---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt
> > index f06f24d405a5..10ae6be7225e 100644
> > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt
> > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt
> > @@ -7,12 +7,13 @@ channel output.
> >
> > Required properties:
> > - compatible: "mediatek,-dsi"
> > -- the supported chips are mt2701, mt7623, mt8173 and mt8183.
> > +- the supported chips are mt2701, mt7623, mt8167, mt8173 and mt8183.
> > - reg: Physical base address and length of the controller's registers
> > - interrupts: The interrupt signal from the function block.
> > - clocks: device clocks
> >See Documentation/devicetree/bindings/clock/clock-bindings.txt for
> > details.
> > -- clock-names: must contain "engine", "digital", and "hs"
> > +- clock-names: must contain "engine", "digital", "hs"
> > + Can optionnally also contain "mipi26m"
>
> It seems that mipi26m is the clock of mipi-tx. In mt8173.dtsi [1],
> mipi-tx's clock is 26m.
>
> mipi_tx0: mipi-dphy@10215000 {
> compatible = "mediatek,mt8173-mipi-tx";
> reg = <0 0x10215000 0 0x1000>;
> clocks = <>;
> clock-output-names = "mipi_tx0_pll";
> #clock-cells = <0>;
> #phy-cells = <0>;
> status = "disabled";
> };
>
> If this is the clock of mipi-tx, it should be controlled by mipi-tx driver.
Thanks, I will fix that in v2.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.9
>
> Regards,
> Chun-Kuang.
>
> > - phys: phandle link to the MIPI D-PHY controller.
> > - phy-names: must contain "dphy"
> > - port: Output port node with endpoint definitions as described in
> > @@ -26,7 +27,7 @@ The MIPI TX configuration module controls the MIPI D-PHY.
> >
> > Required properties:
> > - compatible: "mediatek,-mipi-tx"
> > -- the supported chips are mt2701, 7623, mt8173 and mt8183.
> > +- the supported chips are mt2701, 7623, mt8167, mt8173 and mt8183.
> > - reg: Physical base address and length of the controller's registers
> > - clocks: PLL reference clock
> > - clock-output-names: name of the output clock line to the DSI encoder
> > --
> > 2.28.0
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/sun4i: frontend: Fix the scaler phase on A33
Hi! Dne četrtek, 15. oktober 2020 ob 11:36:42 CEST je Maxime Ripard napisal(a): > The A33 has a different phase parameter in the Allwinner BSP on the > channel1 than the one currently applied. Fix this. > > Signed-off-by: Maxime Ripard Acked-by: Jernej Skrabec Best regards, Jernej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/3] backlight: pwm_bl: Fix interpolation
The previous behavior was a little unexpected, its properties/problems:
1. It was designed to generate strictly increasing values (no repeats)
2. It had quantization errors when calculating step size. Resulting in
unexpected jumps near the end of some segments.
Example settings:
brightness-levels = <0 1 2 4 8 16 32 64 128 256>;
num-interpolated-steps = <16>;
Whenever num-interpolated-steps was larger than the distance
between 2 consecutive brightness levels the table would get really
discontinuous. The slope of the interpolation would stick with
integers only and if it was 0 the whole line segment would get skipped.
The distances between 1 2 4 and 8 would be 1 (property #1 fighting us),
and only starting with 16 it would start to interpolate properly.
Property #1 is not enough. The goal here is more than just monotonically
increasing. We should still care about the shape of the curve. Repeated
points might be desired if we're in the part of the curve where we want
to go slow (aka slope near 0).
Problem #2 is plainly a bug. Imagine if the 64 entry was 63 instead,
the calculated slope on the 32-63 segment will be almost half as it
should be.
The most expected and simplest algorithm for interpolation is linear
interpolation, which would handle both problems.
Let's just implement that!
Take pairs of points from the brightness-levels array and linearly
interpolate between them. On the X axis (what userspace sees) we'll
now have equally sized intervals (num-interpolated-steps sized,
as opposed to before where we were at the mercy of quantization).
END
Signed-off-by: Alexandru Stan
---
drivers/video/backlight/pwm_bl.c | 70 ++--
1 file changed, 31 insertions(+), 39 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index dfc760830eb9..e48fded3e414 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -230,8 +230,7 @@ static int pwm_backlight_parse_dt(struct device *dev,
struct platform_pwm_backlight_data *data)
{
struct device_node *node = dev->of_node;
- unsigned int num_levels = 0;
- unsigned int levels_count;
+ unsigned int num_levels;
unsigned int num_steps = 0;
struct property *prop;
unsigned int *table;
@@ -260,12 +259,11 @@ static int pwm_backlight_parse_dt(struct device *dev,
if (!prop)
return 0;
- data->max_brightness = length / sizeof(u32);
+ num_levels = length / sizeof(u32);
/* read brightness levels from DT property */
- if (data->max_brightness > 0) {
- size_t size = sizeof(*data->levels) * data->max_brightness;
- unsigned int i, j, n = 0;
+ if (num_levels > 0) {
+ size_t size = sizeof(*data->levels) * num_levels;
data->levels = devm_kzalloc(dev, size, GFP_KERNEL);
if (!data->levels)
@@ -273,7 +271,7 @@ static int pwm_backlight_parse_dt(struct device *dev,
ret = of_property_read_u32_array(node, "brightness-levels",
data->levels,
-data->max_brightness);
+num_levels);
if (ret < 0)
return ret;
@@ -298,7 +296,13 @@ static int pwm_backlight_parse_dt(struct device *dev,
* between two points.
*/
if (num_steps) {
- if (data->max_brightness < 2) {
+ unsigned int num_input_levels = num_levels;
+ unsigned int i;
+ u32 x1, x2, x, dx;
+ u32 y1, y2;
+ s64 dy;
+
+ if (num_input_levels < 2) {
dev_err(dev, "can't interpolate\n");
return -EINVAL;
}
@@ -308,14 +312,7 @@ static int pwm_backlight_parse_dt(struct device *dev,
* taking in consideration the number of interpolated
* steps between two levels.
*/
- for (i = 0; i < data->max_brightness - 1; i++) {
- if ((data->levels[i + 1] - data->levels[i]) /
- num_steps)
- num_levels += num_steps;
- else
- num_levels++;
- }
- num_levels++;
+ num_levels = (num_input_levels - 1) * num_steps + 1;
dev_dbg(dev, "new number of brightness levels: %d\n",
num_levels);
@@ -327,24 +324,25 @@ static int pwm_backlight_parse_dt(struct device *dev,
Re: [PATCH] drivers/video: Fix -Wstringop-truncation in hdmi.c
Hi
On 22.10.20 01:06, Laurent Pinchart wrote:
> Hi Thomas,
>
> Thank you for the patch.
>
> On Wed, Oct 21, 2020 at 02:12:41PM +0200, Thomas Zimmermann wrote:
>> Trying to copy into the string fields with strncpy() gives a warning from
>> gcc. Both fields are part of a packed HDMI header and do not require a
>> terminating \0 character.
>>
>> ../drivers/video/hdmi.c: In function 'hdmi_spd_infoframe_init':
>> ../drivers/video/hdmi.c:230:2: warning: 'strncpy' specified bound 8 equals
>> destination size [-Wstringop-truncation]
>> 230 | strncpy(frame->vendor, vendor, sizeof(frame->vendor));
>> | ^
>> ../drivers/video/hdmi.c:231:2: warning: 'strncpy' specified bound 16 equals
>> destination size [-Wstringop-truncation]
>> 231 | strncpy(frame->product, product, sizeof(frame->product));
>> | ^~~~
>>
>> Just use memcpy() instead.
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/video/hdmi.c | 8 ++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
>> index b7a1d6fae90d..1e4cb63d0d11 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -221,14 +221,18 @@ EXPORT_SYMBOL(hdmi_avi_infoframe_pack);
>> int hdmi_spd_infoframe_init(struct hdmi_spd_infoframe *frame,
>> const char *vendor, const char *product)
>> {
>> +size_t len;
>> +
>> memset(frame, 0, sizeof(*frame));
>>
>> frame->type = HDMI_INFOFRAME_TYPE_SPD;
>> frame->version = 1;
>> frame->length = HDMI_SPD_INFOFRAME_SIZE;
>>
>> -strncpy(frame->vendor, vendor, sizeof(frame->vendor));
>> -strncpy(frame->product, product, sizeof(frame->product));
>> +len = strlen(vendor);
>> +memcpy(frame->vendor, vendor, min(len, sizeof(frame->vendor)));
>> +len = strlen(product);
>> +memcpy(frame->product, product, min(len, sizeof(frame->product)));
>
> As this seems to be a legitimate use of strncpy(), isn't there a way to
> silence the warning without requiring this additional runtime complexity
> ?
Yes, the original code this correct. I looked through include/string.h
if there's better string function, but none fits. Most of them
0-terminate the output string.
The only simple fix seems to be to set gcc's -Wno-stringop-truncation
here. I'd expect that would be an even less preferable change.
Best regards
Thomas
>
>>
>> return 0;
>> }
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 12/16] PCI: Obey iomem restrictions for procfs mmap
On Thu, Oct 22, 2020 at 1:20 AM Jason Gunthorpe wrote: > > On Wed, Oct 21, 2020 at 09:24:08PM +0200, Daniel Vetter wrote: > > On Wed, Oct 21, 2020 at 6:37 PM Jason Gunthorpe wrote: > > > > > > On Wed, Oct 21, 2020 at 05:54:54PM +0200, Daniel Vetter wrote: > > > > > > > The trouble is that io_remap_pfn adjust vma->pgoff, so we'd need to > > > > split that. So ideally ->mmap would never set up any ptes. > > > > > > /dev/mem makes pgoff == pfn so it doesn't get changed by remap. > > > > > > pgoff doesn't get touched for MAP_SHARED either, so there are other > > > users that could work like this - eg anyone mmaping IO memory is > > > probably OK. > > > > I was more generally thinking for io_remap_pfn_users because of the > > mkwrite use-case we might have in fbdev emulation in drm. > > You have a use case for MAP_PRIVATE and io_remap_pfn_range()?? Uh no :-) But for ioremaps and keep track of which pages userspace has touched. Problem is that there's many displays where you need to explicitly upload the data, and in drm we have ioctl calls for that. fbdev mmap assumes this just magically happens. So you need to keep track of write faults, launch a delayed worker which first re-protects all ptes and then uploads the dirty pages. And ideally we wouldn't have to implement this everywhere just for fbdev, but could wrap it around an existing mmap implementation by just intercepting mkwrite. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
