| Message ID | 20220219193957.577054-1-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: Avoid dirtyfb stalls on video mode displays | expand |
On 19/02/2022 22:39, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > Someone on IRC once asked an innocent enough sounding question: Why > with xf86-video-modesetting is es2gears limited at 120fps. > > So I broke out the perfetto tracing mesa MR and took a look. It turns > out the problem was drm_atomic_helper_dirtyfb(), which would end up > waiting for vblank.. es2gears would rapidly push two frames to Xorg, > which would blit them to screen and in idle hook (I assume) call the > DIRTYFB ioctl. Which in turn would do an atomic update to flush the > dirty rects, which would stall until the next vblank. And then the > whole process would repeat. > > But this is a bit silly, we only need dirtyfb for command mode DSI > panels. So lets just skip it otherwise. > > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 9 ++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 + > drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 9 ++++ > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 1 + > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 1 + > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 8 +++ > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 1 + > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 1 + > drivers/gpu/drm/msm/msm_fb.c | 64 ++++++++++++++++++++++- > drivers/gpu/drm/msm/msm_kms.h | 2 + > 11 files changed, 109 insertions(+), 1 deletion(-) > I have checked your previous dirtyfb patch (and corresponding discussion). I'm not fond of the idea of acquiring locks, computing the value, then releasing the locks and reacquiring the locks again. I understand the original needs_dirtyfb approach was frowned upon. Do we have a chance of introducing drm_atomic_helper_dirtyfb_unlocked()? Which would perform all the steps of the orignal helper, but without locks acquirement (and state allocation/freeing)? [skipped] > diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c > index 4d34df5354e0..1b0648baeae2 100644 > --- a/drivers/gpu/drm/msm/msm_fb.c > +++ b/drivers/gpu/drm/msm/msm_fb.c > @@ -24,10 +24,72 @@ struct msm_framebuffer { > static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev, > const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos); > > +static int msm_framebuffer_dirtyfb(struct drm_framebuffer *fb, > + struct drm_file *file_priv, unsigned int flags, > + unsigned int color, struct drm_clip_rect *clips, > + unsigned int num_clips) > +{ > + struct msm_drm_private *priv = fb->dev->dev_private; > + struct drm_modeset_acquire_ctx ctx; > + struct drm_plane *plane; > + bool needs_flush = false; > + int ret = 0; > + > + /* > + * When called from ioctl, we are interruptible, but not when called > + * internally (ie. defio worker) > + */ > + drm_modeset_acquire_init(&ctx, > + file_priv ? DRM_MODESET_ACQUIRE_INTERRUPTIBLE : 0); > + > +retry: > + drm_for_each_plane(plane, fb->dev) { > + struct drm_plane_state *plane_state; > + struct drm_crtc *crtc; > + > + ret = drm_modeset_lock(&plane->mutex, &ctx); > + if (ret) > + goto out; > + > + if (plane->state->fb != fb) { > + drm_modeset_unlock(&plane->mutex); > + continue; > + } > + > + crtc = plane->state->crtc; > + > + ret = drm_modeset_lock(&crtc->mutex, &ctx); > + if (ret) > + goto out; > + > + if (priv->kms->funcs->needs_dirtyfb(crtc)) { > + needs_flush = true; > + break; > + } > + } > + > +out: > + if (ret == -EDEADLK) { > + ret = drm_modeset_backoff(&ctx); > + if (!ret) > + goto retry; > + } > + > + drm_modeset_drop_locks(&ctx); > + drm_modeset_acquire_fini(&ctx); > + > + if (needs_flush) { This bit triggers my paranoia. The driver computes the value with the locks being held and then performs some action depending on the computed value after releasing the locks. I'd prefer to acquire modesetting locks for all the planes (and allocate atomic state), check if the dirtyfb processing is needed, then call drm_atomic_helper_dirtyfb_unlocked() withith the same locks section. > + ret = drm_atomic_helper_dirtyfb(fb, file_priv, flags, > + color, clips, num_clips); > + } > + > + return ret; > +} > + > static const struct drm_framebuffer_funcs msm_framebuffer_funcs = { > .create_handle = drm_gem_fb_create_handle, > .destroy = drm_gem_fb_destroy, > - .dirty = drm_atomic_helper_dirtyfb, > + .dirty = msm_framebuffer_dirtyfb, > }; > > #ifdef CONFIG_DEBUG_FS > diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h > index 2a4f0526cb98..eb870d499d1e 100644 > --- a/drivers/gpu/drm/msm/msm_kms.h > +++ b/drivers/gpu/drm/msm/msm_kms.h > @@ -117,6 +117,8 @@ struct msm_kms_funcs { > struct drm_encoder *encoder, > struct drm_encoder *slave_encoder, > bool is_cmd_mode); > + bool (*needs_dirtyfb)(struct drm_crtc *crtc); > + > /* cleanup: */ > void (*destroy)(struct msm_kms *kms); >
On Wed, Feb 23, 2022 at 2:00 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On 19/02/2022 22:39, Rob Clark wrote: > > From: Rob Clark <robdclark@chromium.org> > > > > Someone on IRC once asked an innocent enough sounding question: Why > > with xf86-video-modesetting is es2gears limited at 120fps. > > > > So I broke out the perfetto tracing mesa MR and took a look. It turns > > out the problem was drm_atomic_helper_dirtyfb(), which would end up > > waiting for vblank.. es2gears would rapidly push two frames to Xorg, > > which would blit them to screen and in idle hook (I assume) call the > > DIRTYFB ioctl. Which in turn would do an atomic update to flush the > > dirty rects, which would stall until the next vblank. And then the > > whole process would repeat. > > > > But this is a bit silly, we only need dirtyfb for command mode DSI > > panels. So lets just skip it otherwise. > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +++++ > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 9 ++++ > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 + > > drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 9 ++++ > > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 1 + > > drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 1 + > > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 8 +++ > > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 1 + > > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 1 + > > drivers/gpu/drm/msm/msm_fb.c | 64 ++++++++++++++++++++++- > > drivers/gpu/drm/msm/msm_kms.h | 2 + > > 11 files changed, 109 insertions(+), 1 deletion(-) > > > > I have checked your previous dirtyfb patch (and corresponding discussion). > > I'm not fond of the idea of acquiring locks, computing the value, then > releasing the locks and reacquiring the locks again. I understand the > original needs_dirtyfb approach was frowned upon. Do we have a chance of > introducing drm_atomic_helper_dirtyfb_unlocked()? Which would perform > all the steps of the orignal helper, but without locks acquirement (and > state allocation/freeing)? > The locking is really more just to avoid racing state access with state being free'd. The sort of race you could have is perhaps dirtyfb racing with attaching the fb to a cmd mode plane->crtc->encoder chain. I think this is relatively harmless since that act would flush the fb anyways. But it did give me an idea for a possibly simpler approach.. we might be able to just keep a refcnt of cmd mode panels the fb is indirectly attached to, and then msm_framebuffer_dirtyfb() simply has to check if that count is greater than zero. If we increment/decrement the count in fb->prepare()/cleanup() that should also solve the race. BR, -R
Hi Rob,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next]
[also build test WARNING on drm-intel/for-linux-next drm-tip/drm-tip v5.17-rc5 next-20220223]
[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/Rob-Clark/drm-msm-Avoid-dirtyfb-stalls-on-video-mode-displays/20220220-185344
base: git://anongit.freedesktop.org/drm/drm drm-next
config: mips-randconfig-r021-20220224 (https://download.01.org/0day-ci/archive/20220224/202202241211.iXHxUcF2-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
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
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/0day-ci/linux/commit/e5d737074c502ff040a67f7fe1f8a9e2a308dec9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rob-Clark/drm-msm-Avoid-dirtyfb-stalls-on-video-mode-displays/20220220-185344
git checkout e5d737074c502ff040a67f7fe1f8a9e2a308dec9
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/gpu/drm/msm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/msm/msm_fb.c:47:27: warning: unused variable 'plane_state'
struct drm_plane_state
^
fatal error: error in backend: Nested variants found in inline asm string: ' .set push
.set mips64r2
.if ( 0x00 ) != -1)) 0x00 ) != -1)) : ($( static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = $( .func = __func__, .file = "arch/mips/include/asm/atomic.h", .line = 156, $); 0x00 ) != -1)) : $))) ) && ( 0 ); .set push; .set mips64r2; .rept 1; sync 0x00; .endr; .set pop; .else; ; .endif
1: ll $1, $2 # atomic_fetch_sub
subu $0, $1, $3
sc $0, $2
beqz $0, 1b
.set pop
move $0, $1
'
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0. Program arguments: clang -Wp,-MMD,drivers/gpu/drm/msm/.msm_fb.o.d -nostdinc -Iarch/mips/include -I./arch/mips/include/generated -Iinclude -I./include -Iarch/mips/include/uapi -I./arch/mips/include/generated/uapi -Iinclude/uapi -I./include/generated/uapi -include include/linux/compiler-version.h -include include/linux/kconfig.h -include include/linux/compiler_types.h -D__KERNEL__ -DVMLINUX_LOAD_ADDRESS=0xffffffff84000000 -DLINKER_LOAD_ADDRESS=0x84000000 -DDATAOFFSET=0 -Qunused-arguments -fmacro-prefix-map== -DKBUILD_EXTRA_WARN1 -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89 --target=mipsel-linux -fintegrated-as -Werror=unknown-warning-option -Werror=ignored-optimization-argument -mno-check-zero-division -mabi=32 -G 0 -mno-abicalls -fno-pic -pipe -msoft-float -DGAS_HAS_SET_HARDFLOAT -Wa,-msoft-float -ffreestanding -EL -fno-stack-check -march=mips32 -Wa,--trap -DTOOLCHAIN_SUPPORTS_VIRT -Iarch/mips/include/asm/mach-ralink -Iarch/mips/include/asm/mach-ralink/mt7620 -Iarch/mips/include/asm/mach-generic -fno-asynchronous-unwind-tables -fno-delete-null-pointer-checks -Wno-frame-address -Wno-address-of-packed-member -Os -Wframe-larger-than=1024 -fno-stack-protector -Wimplicit-fallthrough -Wno-gnu -mno-global-merge -Wno-unused-but-set-variable -Wno-unused-const-variable -ftrivial-auto-var-init=pattern -fno-stack-clash-protection -pg -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-array-bounds -fno-strict-overflow -fno-stack-check -Werror=date-time -Werror=incompatible-pointer-types -Wextra -Wunused -Wno-unused-parameter -Wmissing-declarations -Wmissing-format-attribute -Wmissing-prototypes -Wold-style-definition -Wmissing-include-dirs -Wunused-but-set-variable -Wunused-const-variable -Wno-missing-field-initializers -Wno-sign-compare -Wno-type-limits -I drivers/gpu/drm/msm -I drivers/gpu/drm/msm/disp/dpu1 -I drivers/gpu/drm/msm -I ./drivers/gpu/drm/msm -ffunction-sections -fdata-sections -DKBUILD_MODFILE="drivers/gpu/drm/msm/msm" -DKBUILD_BASENAME="msm_fb" -DKBUILD_MODNAME="msm" -D__KBUILD_MODNAME=kmod_msm -c -o drivers/gpu/drm/msm/msm_fb.o drivers/gpu/drm/msm/msm_fb.c
1. <eof> parser at end of file
2. Code generation
3. Running pass 'Function Pass Manager' on module 'drivers/gpu/drm/msm/msm_fb.c'.
4. Running pass 'Mips Assembly Printer' on function '@msm_framebuffer_create'
#0 0x000055e0318e4d7f Signals.cpp:0:0
#1 0x000055e0318e2c5c llvm::sys::CleanupOnSignal(unsigned long) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x348ec5c)
#2 0x000055e031822fd7 llvm::CrashRecoveryContext::HandleExit(int) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x33cefd7)
#3 0x000055e0318db30e llvm::sys::Process::Exit(int, bool) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x348730e)
#4 0x000055e02f506ccb (/opt/cross/clang-d271fc04d5/bin/clang-15+0x10b2ccb)
#5 0x000055e031829a8c llvm::report_fatal_error(llvm::Twine const&, bool) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x33d5a8c)
#6 0x000055e0325335c0 llvm::AsmPrinter::emitInlineAsm(llvm::MachineInstr const (/opt/cross/clang-d271fc04d5/bin/clang-15+0x40df5c0)
#7 0x000055e03252f4f4 llvm::AsmPrinter::emitFunctionBody() (/opt/cross/clang-d271fc04d5/bin/clang-15+0x40db4f4)
#8 0x000055e02ff70887 llvm::MipsAsmPrinter::runOnMachineFunction(llvm::MachineFunction&) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x1b1c887)
#9 0x000055e030c2d54d llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (.part.53) MachineFunctionPass.cpp:0:0
#10 0x000055e031074807 llvm::FPPassManager::runOnFunction(llvm::Function&) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x2c20807)
#11 0x000055e031074981 llvm::FPPassManager::runOnModule(llvm::Module&) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x2c20981)
#12 0x000055e0310754ff llvm::legacy::PassManagerImpl::run(llvm::Module&) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x2c214ff)
#13 0x000055e031bff147 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x37ab147)
#14 0x000055e03284d693 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x43f9693)
#15 0x000055e0333286e9 clang::ParseAST(clang::Sema&, bool, bool) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x4ed46e9)
#16 0x000055e03284c4cf clang::CodeGenAction::ExecuteAction() (/opt/cross/clang-d271fc04d5/bin/clang-15+0x43f84cf)
#17 0x000055e03224f561 clang::FrontendAction::Execute() (/opt/cross/clang-d271fc04d5/bin/clang-15+0x3dfb561)
#18 0x000055e0321e5faa clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x3d91faa)
#19 0x000055e032313cbb (/opt/cross/clang-d271fc04d5/bin/clang-15+0x3ebfcbb)
#20 0x000055e02f50827c cc1_main(llvm::ArrayRef<char char (/opt/cross/clang-d271fc04d5/bin/clang-15+0x10b427c)
#21 0x000055e02f504f4b ExecuteCC1Tool(llvm::SmallVectorImpl<char driver.cpp:0:0
#22 0x000055e03207ed95 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> const::'lambda'()>(long) Job.cpp:0:0
#23 0x000055e031822e93 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x33cee93)
#24 0x000055e03207f68e clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> const (.part.216) Job.cpp:0:0
#25 0x000055e032054267 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const (/opt/cross/clang-d271fc04d5/bin/clang-15+0x3c00267)
#26 0x000055e032054c47 clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command >&) const (/opt/cross/clang-d271fc04d5/bin/clang-15+0x3c00c47)
#27 0x000055e03205e2f9 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command >&) (/opt/cross/clang-d271fc04d5/bin/clang-15+0x3c0a2f9)
#28 0x000055e02f42d63f main (/opt/cross/clang-d271fc04d5/bin/clang-15+0xfd963f)
#29 0x00007f8e07c7ed0a __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26d0a)
#30 0x000055e02f504a6a _start (/opt/cross/clang-d271fc04d5/bin/clang-15+0x10b0a6a)
clang-15: error: clang frontend command failed with exit code 70 (use -v to see invocation)
clang version 15.0.0 (git://gitmirror/llvm_project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
Target: mipsel-unknown-linux
Thread model: posix
InstalledDir: /opt/cross/clang-d271fc04d5/bin
clang-15: note: diagnostic msg:
Makefile arch drivers fs include kernel nr_bisected scripts source usr
vim +/plane_state +47 drivers/gpu/drm/msm/msm_fb.c
23
24 static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev,
25 const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos);
26
27 static int msm_framebuffer_dirtyfb(struct drm_framebuffer *fb,
28 struct drm_file *file_priv, unsigned int flags,
29 unsigned int color, struct drm_clip_rect *clips,
30 unsigned int num_clips)
31 {
32 struct msm_drm_private *priv = fb->dev->dev_private;
33 struct drm_modeset_acquire_ctx ctx;
34 struct drm_plane *plane;
35 bool needs_flush = false;
36 int ret = 0;
37
38 /*
39 * When called from ioctl, we are interruptible, but not when called
40 * internally (ie. defio worker)
41 */
42 drm_modeset_acquire_init(&ctx,
43 file_priv ? DRM_MODESET_ACQUIRE_INTERRUPTIBLE : 0);
44
45 retry:
46 drm_for_each_plane(plane, fb->dev) {
> 47 struct drm_plane_state *plane_state;
48 struct drm_crtc *crtc;
49
50 ret = drm_modeset_lock(&plane->mutex, &ctx);
51 if (ret)
52 goto out;
53
54 if (plane->state->fb != fb) {
55 drm_modeset_unlock(&plane->mutex);
56 continue;
57 }
58
59 crtc = plane->state->crtc;
60
61 ret = drm_modeset_lock(&crtc->mutex, &ctx);
62 if (ret)
63 goto out;
64
65 if (priv->kms->funcs->needs_dirtyfb(crtc)) {
66 needs_flush = true;
67 break;
68 }
69 }
70
71 out:
72 if (ret == -EDEADLK) {
73 ret = drm_modeset_backoff(&ctx);
74 if (!ret)
75 goto retry;
76 }
77
78 drm_modeset_drop_locks(&ctx);
79 drm_modeset_acquire_fini(&ctx);
80
81 if (needs_flush) {
82 ret = drm_atomic_helper_dirtyfb(fb, file_priv, flags,
83 color, clips, num_clips);
84 }
85
86 return ret;
87 }
88
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index e7c9fe1a250f..3706053bc164 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -265,6 +265,19 @@ static bool dpu_crtc_get_scanout_position(struct drm_crtc *crtc, return true; } +bool dpu_crtc_needs_dirtyfb(struct drm_crtc *crtc) +{ + struct drm_encoder *encoder; + + drm_for_each_encoder_mask (encoder, crtc->dev, crtc->state->encoder_mask) { + if (dpu_encoder_get_intf_mode(encoder) == INTF_MODE_CMD) { + return true; + } + } + + return false; +} + static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer, struct dpu_plane_state *pstate, struct dpu_format *format) { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index b8785c394fcc..64f79b22aba7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -261,6 +261,15 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc); */ void dpu_crtc_complete_commit(struct drm_crtc *crtc); +/** + * dpu_crtc_needs_dirtyfb - do fb updates need to be flushed + * @crtc: Pointer to drm crtc object + * + * Return whether front-buffer updates need to be flushed, ie. is it + * a command-mode style of display + */ +bool dpu_crtc_needs_dirtyfb(struct drm_crtc *crtc); + /** * dpu_crtc_init - create a new crtc object * @dev: dpu device diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 47fe11a84a77..84d9521e8013 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -949,6 +949,7 @@ static const struct msm_kms_funcs kms_funcs = { .check_modified_format = dpu_format_check_modified_format, .get_format = dpu_get_msm_format, .round_pixclk = dpu_kms_round_pixclk, + .needs_dirtyfb = dpu_crtc_needs_dirtyfb, .destroy = dpu_kms_destroy, .snapshot = dpu_kms_mdp_snapshot, #ifdef CONFIG_DEBUG_FS diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c index 169f9de4a12a..64ff21cfed03 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c @@ -19,6 +19,7 @@ struct mdp4_crtc { int id; int ovlp; enum mdp4_dma dma; + enum mdp4_intf intf; bool enabled; /* which mixer/encoder we route output to: */ @@ -594,6 +595,7 @@ void mdp4_crtc_set_intf(struct drm_crtc *crtc, enum mdp4_intf intf, int mixer) intf_sel |= MDP4_DISP_INTF_SEL_DSI_CMD; } + mdp4_crtc->intf = intf; mdp4_crtc->mixer = mixer; blend_setup(crtc); @@ -612,6 +614,13 @@ void mdp4_crtc_wait_for_commit_done(struct drm_crtc *crtc) mdp4_crtc_wait_for_flush_done(crtc); } +bool mdp4_crtc_needs_dirtyfb(struct drm_crtc *crtc) +{ + struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc); + + return mdp4_crtc->intf == INTF_DSI_CMD; +} + static const char *dma_names[] = { "DMA_P", "DMA_S", "DMA_E", }; diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c index 5a33bb148e9e..5e1d19df5c18 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c @@ -163,6 +163,7 @@ static const struct mdp_kms_funcs kms_funcs = { .complete_commit = mdp4_complete_commit, .get_format = mdp_get_format, .round_pixclk = mdp4_round_pixclk, + .needs_dirtyfb = mdp4_crtc_needs_dirtyfb, .destroy = mdp4_destroy, }, .set_irqmask = mdp4_set_irqmask, diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h index e8ee92ab7956..1633ec866b49 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h @@ -190,6 +190,7 @@ uint32_t mdp4_crtc_vblank(struct drm_crtc *crtc); void mdp4_crtc_set_config(struct drm_crtc *crtc, uint32_t config); void mdp4_crtc_set_intf(struct drm_crtc *crtc, enum mdp4_intf intf, int mixer); void mdp4_crtc_wait_for_commit_done(struct drm_crtc *crtc); +bool mdp4_crtc_needs_dirtyfb(struct drm_crtc *crtc); struct drm_crtc *mdp4_crtc_init(struct drm_device *dev, struct drm_plane *plane, int id, int ovlp_id, enum mdp4_dma dma_id); diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c index bb7d066618e6..fac2cfd3ef7e 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c @@ -1300,6 +1300,14 @@ void mdp5_crtc_wait_for_commit_done(struct drm_crtc *crtc) mdp5_crtc_wait_for_flush_done(crtc); } +bool mdp5_crtc_needs_dirtyfb(struct drm_crtc *crtc) +{ + struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state); + struct mdp5_interface *intf = mdp5_cstate->pipeline.intf; + + return intf->mode == MDP5_INTF_DSI_MODE_COMMAND; +} + /* initialize crtc */ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev, struct drm_plane *plane, diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 12a5f81e402b..25a474732e7f 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -280,6 +280,7 @@ static const struct mdp_kms_funcs kms_funcs = { .get_format = mdp_get_format, .round_pixclk = mdp5_round_pixclk, .set_split_display = mdp5_set_split_display, + .needs_dirtyfb = mdp5_crtc_needs_dirtyfb, .destroy = mdp5_kms_destroy, #ifdef CONFIG_DEBUG_FS .debugfs_init = mdp5_kms_debugfs_init, diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h index ac269a6802df..ae62f6778346 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h @@ -280,6 +280,7 @@ struct mdp5_hw_mixer *mdp5_crtc_get_mixer(struct drm_crtc *crtc); struct mdp5_pipeline *mdp5_crtc_get_pipeline(struct drm_crtc *crtc); void mdp5_crtc_set_pipeline(struct drm_crtc *crtc); void mdp5_crtc_wait_for_commit_done(struct drm_crtc *crtc); +bool mdp5_crtc_needs_dirtyfb(struct drm_crtc *crtc); struct drm_crtc *mdp5_crtc_init(struct drm_device *dev, struct drm_plane *plane, struct drm_plane *cursor_plane, int id); diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c index 4d34df5354e0..1b0648baeae2 100644 --- a/drivers/gpu/drm/msm/msm_fb.c +++ b/drivers/gpu/drm/msm/msm_fb.c @@ -24,10 +24,72 @@ struct msm_framebuffer { static struct drm_framebuffer *msm_framebuffer_init(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos); +static int msm_framebuffer_dirtyfb(struct drm_framebuffer *fb, + struct drm_file *file_priv, unsigned int flags, + unsigned int color, struct drm_clip_rect *clips, + unsigned int num_clips) +{ + struct msm_drm_private *priv = fb->dev->dev_private; + struct drm_modeset_acquire_ctx ctx; + struct drm_plane *plane; + bool needs_flush = false; + int ret = 0; + + /* + * When called from ioctl, we are interruptible, but not when called + * internally (ie. defio worker) + */ + drm_modeset_acquire_init(&ctx, + file_priv ? DRM_MODESET_ACQUIRE_INTERRUPTIBLE : 0); + +retry: + drm_for_each_plane(plane, fb->dev) { + struct drm_plane_state *plane_state; + struct drm_crtc *crtc; + + ret = drm_modeset_lock(&plane->mutex, &ctx); + if (ret) + goto out; + + if (plane->state->fb != fb) { + drm_modeset_unlock(&plane->mutex); + continue; + } + + crtc = plane->state->crtc; + + ret = drm_modeset_lock(&crtc->mutex, &ctx); + if (ret) + goto out; + + if (priv->kms->funcs->needs_dirtyfb(crtc)) { + needs_flush = true; + break; + } + } + +out: + if (ret == -EDEADLK) { + ret = drm_modeset_backoff(&ctx); + if (!ret) + goto retry; + } + + drm_modeset_drop_locks(&ctx); + drm_modeset_acquire_fini(&ctx); + + if (needs_flush) { + ret = drm_atomic_helper_dirtyfb(fb, file_priv, flags, + color, clips, num_clips); + } + + return ret; +} + static const struct drm_framebuffer_funcs msm_framebuffer_funcs = { .create_handle = drm_gem_fb_create_handle, .destroy = drm_gem_fb_destroy, - .dirty = drm_atomic_helper_dirtyfb, + .dirty = msm_framebuffer_dirtyfb, }; #ifdef CONFIG_DEBUG_FS diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index 2a4f0526cb98..eb870d499d1e 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -117,6 +117,8 @@ struct msm_kms_funcs { struct drm_encoder *encoder, struct drm_encoder *slave_encoder, bool is_cmd_mode); + bool (*needs_dirtyfb)(struct drm_crtc *crtc); + /* cleanup: */ void (*destroy)(struct msm_kms *kms);