As per the FIXME this code is entirely duplicate with what is already
provided inside drm_dsc_compute_rc_parameters(), supposedly because that
function was yielding "incorrect" results while in reality the panel
driver(s?) used for testing were providing incorrect parameters.
For example, this code from downstream assumed dsc->bits_per_pixel to
contain an integer value, whereas the upstream drm_dsc_config struct
stores it with 4 fractional bits. drm_dsc_compute_rc_parameters()
already accounts for this feat while the panel driver used for testing
[1] wasn't, hence making drm_dsc_compute_rc_parameters() seem like it
was returning an incorrect result.
Other users of dsc->bits_per_pixel inside dsi_populate_dsc_params() also
treat it in the same erroneous way, and will be addressed in a separate
patch.
In the end, using drm_dsc_compute_rc_parameters() spares both a lot of
duplicate code and erratic behaviour.
[1]: https://git.linaro.org/people/vinod.koul/kernel.git/commit/?h=topic/pixel3_5.18-rc1&id=1d7d98ad564f1ec69e7525e07418918d90f247a1
Fixes: b9080324d6 ("drm/msm/dsi: add support for dsc data")
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Patchwork: https://patchwork.freedesktop.org/patch/508939/
Link: https://lore.kernel.org/r/20221026182824.876933-7-marijn.suijten@somainline.org
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
This field is currently unread but will come into effect when duplicated
code below is migrated to call drm_dsc_compute_rc_parameters(), which
uses the bpc-dependent value of the local variable mux_words_size in
much the same way.
The hardcoded constant seems to be a remnant from the `/* bpc 8 */`
comment right above, indicating that this group of field assignments is
applicable to bpc = 8 exclusively and should probably bail out on
different bpc values, until constants for other bpc values are added (or
the current ones are confirmed to be correct across multiple bpc's).
Fixes: b9080324d6 ("drm/msm/dsi: add support for dsc data")
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
Patchwork: https://patchwork.freedesktop.org/patch/508943/
Link: https://lore.kernel.org/r/20221026182824.876933-6-marijn.suijten@somainline.org
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
According to the comment this DPU register contains the bits per pixel
as a 6.4 fractional value, conveniently matching the contents of
bits_per_pixel in struct drm_dsc_config which also uses 4 fractional
bits. However, the downstream source this implementation was
copy-pasted from has its bpp field stored _without_ fractional part.
This makes the entire convoluted math obsolete as it is impossible to
pull those 4 fractional bits out of thin air, by somehow trying to reuse
the lowest 2 bits of a non-fractional bpp (lsb = bpp % 4??).
The rest of the code merely attempts to keep the integer part a multiple
of 4, which is rendered useless thanks to data |= dsc->bits_per_pixel <<
12; already filling up those bits anyway (but not on downstream).
Fixes: c110cfd175 ("drm/msm/disp/dpu1: Add support for DSC")
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
Patchwork: https://patchwork.freedesktop.org/patch/508946/
Link: https://lore.kernel.org/r/20221026182824.876933-10-marijn.suijten@somainline.org
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
The fb_base in struct drm_mode_config has been unused for a long time.
Some drivers set it and some don't leading to a very confusing state
where the variable can't be relied upon, because there's no indication
as to which driver sets it and which doesn't.
The only usage of fb_base is internal to two drivers so instead of trying
to force it into all the drivers to get it into a coherent state
completely remove it.
Signed-off-by: Zack Rusin <zackr@vmware.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Thomas Zimmermann <tzimemrmann@suse.de>
Acked-by: Christian König <christian.koenig@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221019024401.394617-1-zack@kde.org
DOWNSPREAD_CTRL (0x107) shall be cleared to 0 upon power-on reset or an
upstream device disconnect. This patch will enforce this rule by always
cleared DOWNSPREAD_CTRL register to 0 before start link training. At rare
case that DP MSA timing parameters may be mis-interpreted by the sink
which causes audio sampling rate be calculated wrongly and cause audio
did not work at sink if DOWNSPREAD_CTRL register is not cleared to 0.
Changes in v2:
1) fix spelling at commit text
2) merge ssc variable into encoding[0]
Changes in v3:
-- correct spelling of DOWNSPREAD_CTRL
-- replace err with len of ssize_t
Changes in v4:
-- split into 2 patches
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Fixes: c943b4948b ("drm/msm/dp: add displayPort driver support")
Patchwork: https://patchwork.freedesktop.org/patch/502532/
Link: https://lore.kernel.org/r/1662999830-13916-2-git-send-email-quic_khsieh@quicinc.com
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.
This can lead resource leaks or failure to bind the aggregate device
when binding is later retried and a second attempt to allocate the
resources is made.
For the DP aux-bus, an attempt to populate the bus a second time will
simply fail ("DP AUX EP device already populated").
Fix this by tying the lifetime of the EP device to the DRM device rather
than DP controller platform device.
Fixes: c3bf8e21b3 ("drm/msm/dp: Add eDP support via aux_bus")
Cc: stable@vger.kernel.org # 5.19
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/502672/
Link: https://lore.kernel.org/r/20220913085320.8577-7-johan+linaro@kernel.org
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Device-managed resources allocated post component bind must be tied to
the lifetime of the aggregate DRM device or they will not necessarily be
released when binding of the aggregate device is deferred.
This is specifically true for the DP IRQ, which will otherwise remain
requested so that the next bind attempt fails when requesting the IRQ a
second time.
Since commit c3bf8e21b3 ("drm/msm/dp: Add eDP support via aux_bus")
this can happen when the aux-bus panel driver has not yet been loaded so
that probe is deferred.
Fix this by tying the device-managed lifetime of the DP IRQ to the DRM
device so that it is released when bind fails.
Fixes: c943b4948b ("drm/msm/dp: add displayPort driver support")
Cc: stable@vger.kernel.org # 5.10
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/502679/
Link: https://lore.kernel.org/r/20220913085320.8577-6-johan+linaro@kernel.org
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
DRM commit_tails() will disable downstream crtc/encoder/bridge if
both disable crtc is required and crtc->active is set before pushing
a new frame downstream.
There is a rare case that user space display manager issue an extra
screen update immediately followed by close DRM device while down
stream display interface is disabled. This extra screen update will
timeout due to the downstream interface is disabled but will cause
crtc->active be set. Hence the followed commit_tails() called by
drm_release() will pass the disable downstream crtc/encoder/bridge
conditions checking even downstream interface is disabled.
This cause the crash to happen at dp_bridge_disable() due to it trying
to access the main link register to push the idle pattern out while main
link clocks is disabled.
This patch adds atomic_check to prevent the extra frame will not
be pushed down if display interface is down so that crtc->active
will not be set neither. This will fail the conditions checking
of disabling down stream crtc/encoder/bridge which prevent
drm_release() from calling dp_bridge_disable() so that crash
at dp_bridge_disable() prevented.
There is no protection in the DRM framework to check if the display
pipeline has been already disabled before trying again. The only
check is the crtc_state->active but this is controlled by usermode
using UAPI. Hence if the usermode sets this and then crashes, the
driver needs to protect against double disable.
SError Interrupt on CPU7, code 0x00000000be000411 -- SError
CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
Hardware name: Google Lazor (rev3 - 8) (DT)
pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __cmpxchg_case_acq_32+0x14/0x2c
lr : do_raw_spin_lock+0xa4/0xdc
sp : ffffffc01092b6a0
x29: ffffffc01092b6a0 x28: 0000000000000028 x27: 0000000000000038
x26: 0000000000000004 x25: ffffffd2973dce48 x24: 0000000000000000
x23: 00000000ffffffff x22: 00000000ffffffff x21: ffffffd2978d0008
x20: ffffffd2978d0008 x19: ffffff80ff759fc0 x18: 0000000000000000
x17: 004800a501260460 x16: 0441043b04600438 x15: 04380000089807d0
x14: 07b0089807800780 x13: 0000000000000000 x12: 0000000000000000
x11: 0000000000000438 x10: 00000000000007d0 x9 : ffffffd2973e09e4
x8 : ffffff8092d53300 x7 : ffffff808902e8b8 x6 : 0000000000000001
x5 : ffffff808902e880 x4 : 0000000000000000 x3 : ffffff80ff759fc0
x2 : 0000000000000001 x1 : 0000000000000000 x0 : ffffff80ff759fc0
Kernel panic - not syncing: Asynchronous SError Interrupt
CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
Hardware name: Google Lazor (rev3 - 8) (DT)
Call trace:
dump_backtrace.part.0+0xbc/0xe4
show_stack+0x24/0x70
dump_stack_lvl+0x68/0x84
dump_stack+0x18/0x34
panic+0x14c/0x32c
nmi_panic+0x58/0x7c
arm64_serror_panic+0x78/0x84
do_serror+0x40/0x64
el1h_64_error_handler+0x30/0x48
el1h_64_error+0x68/0x6c
__cmpxchg_case_acq_32+0x14/0x2c
_raw_spin_lock_irqsave+0x38/0x4c
lock_timer_base+0x40/0x78
__mod_timer+0xf4/0x25c
schedule_timeout+0xd4/0xfc
__wait_for_common+0xac/0x140
wait_for_completion_timeout+0x2c/0x54
dp_ctrl_push_idle+0x40/0x88
dp_bridge_disable+0x24/0x30
drm_atomic_bridge_chain_disable+0x90/0xbc
drm_atomic_helper_commit_modeset_disables+0x198/0x444
msm_atomic_commit_tail+0x1d0/0x374
commit_tail+0x80/0x108
drm_atomic_helper_commit+0x118/0x11c
drm_atomic_commit+0xb4/0xe0
drm_client_modeset_commit_atomic+0x184/0x224
drm_client_modeset_commit_locked+0x58/0x160
drm_client_modeset_commit+0x3c/0x64
__drm_fb_helper_restore_fbdev_mode_unlocked+0x98/0xac
drm_fb_helper_set_par+0x74/0x80
drm_fb_helper_hotplug_event+0xdc/0xe0
__drm_fb_helper_restore_fbdev_mode_unlocked+0x7c/0xac
drm_fb_helper_restore_fbdev_mode_unlocked+0x20/0x2c
drm_fb_helper_lastclose+0x20/0x2c
drm_lastclose+0x44/0x6c
drm_release+0x88/0xd4
__fput+0x104/0x220
____fput+0x1c/0x28
task_work_run+0x8c/0x100
do_exit+0x450/0x8d0
do_group_exit+0x40/0xac
__wake_up_parent+0x0/0x38
invoke_syscall+0x84/0x11c
el0_svc_common.constprop.0+0xb8/0xe4
do_el0_svc+0x8c/0xb8
el0_svc+0x2c/0x54
el0t_64_sync_handler+0x120/0x1c0
el0t_64_sync+0x190/0x194
SMP: stopping secondary CPUs
Kernel Offset: 0x128e800000 from 0xffffffc008000000
PHYS_OFFSET: 0x80000000
CPU features: 0x800,00c2a015,19801c82
Memory Limit: none
Changes in v2:
-- add more commit text
Changes in v3:
-- add comments into dp_bridge_atomic_check()
Changes in v4:
-- rewording the comment into dp_bridge_atomic_check()
Changes in v5:
-- removed quote x at end of commit text
Changes in v6:
-- removed quote x at end of comment in dp_bridge_atomic_check()
Fixes: 8a3b4c17f8 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
Reported-by: Leonard Lausen <leonard@lausen.nl>
Suggested-by: Rob Clark <robdclark@gmail.com>
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/17
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/505331/
Link: https://lore.kernel.org/r/1664408211-25314-1-git-send-email-quic_khsieh@quicinc.com
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
The introduction of "drm/msm/gem: Evict active GEM objects when necessary"
exposes a problem with "drm/msm/gem: Unpin buffers earlier", in that we
need to keep the object pinned in the time the submit is queued up in the
gpu scheduler. Otherwise the shrinker will see it as a thing that can be
evicted if we wait for it to be signaled. But if the shrinker path is
waiting on it with the obj lock held, the job cannot be scheduled, as that
also requires briefly grabbing the obj lock, leading to deadlock. (Not to
mention, we don't want the shrinker to evict an obj queued up in gpu
scheduler.)
Fixes: f371bcc0c2 ("drm/msm/gem: Unpin buffers earlier")
Fixes: 025d27239a ("drm/msm/gem: Evict active GEM objects when necessary")
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/19
Signed-off-by: Rob Clark <robdclark@chromium.org>
Tested-by: Chia-I Wu <olvaffe@gmail.com>
Patchwork: https://patchwork.freedesktop.org/patch/504528/
Link: https://lore.kernel.org/r/20220923224043.2449152-1-robdclark@gmail.com
msm-next for v6.1
DPU:
- simplified VBIF configuration
- cleaned up CTL interfaces to accept indices rather than flush masks
DSI:
- removed unused msm_display_dsc_config struct
- switch regulator calls to new bulk API
- switched to use PANEL_BRIDGE for directly attached panels
DSI PHY:
- converted drivers to use parent_hws instead of parent_names
DP:
- cleaned up pixel_rate handling
HDMI PHY:
- turned hdmi-phy-8996 into OF clk provider
core:
- misc dt-bindings fixes
- choose eDP as primary display if it's available
- support getting interconnects from either the mdss or the mdp5/dpu
device nodes
gpu+gem:
- Shrinker + LRU re-work:
- adds a shared GEM LRU+shrinker helper and moves msm over to that
- reduces lock contention between retire and submit by avoiding the
need to acquire obj lock in retire path (and instead using resv
seeing obj's busyness in the shrinker
- fix reclaim vs submit issues
- GEM fault injection for triggering userspace error paths
- Map/unmap optimization
- Improved robustness for a6xx GPU recovery
Signed-off-by: Dave Airlie <airlied@redhat.com>
From: Rob Clark <robdclark@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/CAF6AEGsrfrr9v1oR9S4oYfOs9jm=jbKQiwPBTrCRHrjYerJJFA@mail.gmail.com
