I'm embarassed to say that even though I've touched
vop_crtc_mode_fixup() twice and I swear I tested it, there's still a
stupid glaring bug in it. Specifically, on veyron_minnie (with all
the latest display timings) we want to be setting our pixel clock to
66,666,666.67 Hz and we tell userspace that's what we set, but we're
actually choosing 66,000,000 Hz. This is confirmed by looking at the
clock tree.
The problem is that in drm_display_mode_from_videomode() we convert
from Hz to kHz with:
dmode->clock = vm->pixelclock / 1000;
...and drm_display_mode_from_videomode() is called from panel-simple
when we have an "override_mode" like we do on veyron_minnie. See
commit 123643e5c4 ("ARM: dts: rockchip: Specify
rk3288-veyron-minnie's display timings").
...so when the device tree specifies a clock of 66666667 for the panel
then DRM translates that to 66666000. The clock framework will always
pick a clock that is _lower_ than the one requested, so it will refuse
to pick 66666667 and we'll end up at 66000000.
While we could try to fix drm_display_mode_from_videomode() to round
to the nearest kHz and it would fix our problem, it wouldn't help if
the clock we actually needed was 60,000,001 Hz. We could
alternatively have DRM always round up, but maybe this would break
someone else who already baked in the assumption that DRM rounds down.
Specifically note that clock drivers are not consistent about whether
they round up or round down when you call clk_set_rate(). We know how
Rockchip's clock driver works, but (for instance) you can see that on
most Qualcomm clocks the default is clk_rcg2_ops which rounds up.
Let's solve this by just adding 999 Hz before calling
clk_round_rate(). This should be safe and work everywhere. As
discussed in more detail in comments in the commit, Rockchip's PLLs
are configured in a way that there shouldn't be another PLL setting
that is only a few kHz off so we won't get mixed up.
NOTE: if this is picked to stable, it's probably easiest to first pick
commit 527e4ca3b6 ("drm/rockchip: Base adjustments of the mode based
on prev adjustments") which shouldn't hurt in stable.
Fixes: b59b8de314 ("drm/rockchip: return a true clock rate to adjusted_mode")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20191003114726.v2.1.Ib233b3e706cf6317858384264d5b0ed35657456e@changeid
Panfrost uses multiple schedulers (one for each slot, so 2 in reality),
and on a timeout has to stop all the schedulers to safely perform a
reset. However more than one scheduler can trigger a timeout at the same
time. This race condition results in jobs being freed while they are
still in use.
When stopping other slots use cancel_delayed_work_sync() to ensure that
any timeout started for that slot has completed. Also use
mutex_trylock() to obtain reset_lock. This means that only one thread
attempts the reset, the other threads will simply complete without doing
anything (the first thread will wait for this in the call to
cancel_delayed_work_sync()).
While we're here and since the function is already dependent on
sched_job not being NULL, let's remove the unnecessary checks.
Fixes: aa20236784 ("drm/panfrost: Prevent concurrent resets")
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Steven Price <steven.price@arm.com>
Signed-off-by: Rob Herring <robh@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20191009094456.9704-1-steven.price@arm.com
Currently the property docs don't specify whether it's okay for two planes to
have the same zpos value and what user-space should expect in this case.
The unspoken, legacy rule used in the past was to make user-space figure
out the zpos from object IDs. However some drivers break this rule,
that's why the ordering is documented as unspecified in case the zpos
property is missing. User-space should rely on the zpos property only.
There are some cases in which user-space might read identical zpos
values for different planes.
For instance, in case the property is mutable, user-space might set two
planes' zpos to the same value. This is necessary to support user-space
using the legacy DRM API where atomic commits are not possible:
user-space needs to update the planes' zpos one by one.
Because of this, user-space should handle multiple planes with the same
zpos.
While at it, remove the assumption that zpos is only for overlay planes.
Additionally, update the drm_plane_state.zpos docs to clarify that zpos
disambiguation via plane object IDs is a recommendation for drivers, not
something user-space can rely on. In other words, when user-space sets
the same zpos on two planes, drivers should rely on the plane object ID.
v2: clarify drm_plane_state.zpos docs (Daniel)
v3: zpos is for all planes (Marius, Daniel)
v4: completely reword the drm_plane_state.zpos docs to make it clear the
recommendation to use plane IDs is for drivers in case user-space uses
duplicate zpos values (Pekka)
v5: reword commit message (Pekka, James)
v6: remove mention of Arm GPUs having planes which can't overlap,
because this isn't uAPI yet (Daniel)
Signed-off-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Marius Vlad <marius.vlad@collabora.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: James Qian Wang <james.qian.wang@arm.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/T5nHrvXH0GKOp6ONaFHk-j2cwEb4_4C_sBz9rNw8mmPACuut-DQqC74HMAFKZH3_Q15E8a3YnmKCxap-djKA71VVZv_T-tFxaB0he13O7yA=@emersion.fr
Refer to EDID 1.3 spec, display FEATURE (byte 18h) bit #0 said
"If this bit is set to 1, the display supports timings based on the
GTF standard using default GTF parameter values".
And EDID 1.4 spec shows "If bit 0 is set to 0, then the display
is noncontinuous frequency (multi-mode) and is only specified to accept
the video timing formats that are listed in BASE EDID and certain
EXTENSION Blocks.
When display feature did not support CVT or GFT2 and monitor's EDID version
greater than or equal to "1.2". DRM driver would select GTF as default
for standard timing calculation. It may generated some video timing
that can't display properly by external monitor.
For example. When driver retrieved "0xD1 0xFC" (FHD, 120Hz) and
"0xD1 0xE8" (FHD, 100Hz) from "Standard Timings". GTF formula
would generate video timing like below. It already over monitor's
spec to cause black screen issue.
"1920x1080" 120 368881 1920 2072 2288 2656 1080 1081 1084 1157 0x0 0x6
"1920x1080" 100 301992 1920 2072 2280 2640 1080 1081 1084 1144 0x0 0x6
v2: Just confirm GTF flag and omit the revision check.
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Adam Jackson <ajax@redhat.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191007135127.9538-1-shawn.c.lee@intel.com
For historical reasons, the function drm_wait_vblank_ioctl always return
-EINVAL if something gets wrong. This scenario limits the flexibility
for the userspace to make detailed verification of any problem and take
some action. In particular, the validation of “if (!dev->irq_enabled)”
in the drm_wait_vblank_ioctl is responsible for checking if the driver
support vblank or not. If the driver does not support VBlank, the
function drm_wait_vblank_ioctl returns EINVAL, which does not represent
the real issue; this patch changes this behavior by return EOPNOTSUPP.
Additionally, drm_crtc_get_sequence_ioctl and
drm_crtc_queue_sequence_ioctl, also returns EINVAL if vblank is not
supported; this patch also changes the return value to EOPNOTSUPP in
these functions. Lastly, these functions are invoked by libdrm, which is
used by many compositors; because of this, it is important to check if
this change breaks any compositor. In this sense, the following projects
were examined:
* Drm-hwcomposer
* Kwin
* Sway
* Wlroots
* Wayland
* Weston
* Mutter
* Xorg (67 different drivers)
For each repository the verification happened in three steps:
* Update the main branch
* Look for any occurrence of "drmCrtcQueueSequence",
"drmCrtcGetSequence", and "drmWaitVBlank" with the command git grep -n
"STRING".
* Look in the git history of the project with the command
git log -S<STRING>
None of the above projects validate the use of EINVAL when using
drmWaitVBlank(), which make safe, at least for these projects, to change
the return values. On the other hand, mesa and xserver project uses
drmCrtcQueueSequence() and drmCrtcGetSequence(); this change is harmless
for both projects.
Change since V5 (Pekka Paalanen):
- Check if the change also affects Mutter
Change since V4 (Daniel):
- Also return EOPNOTSUPP in drm_crtc_[get|queue]_sequence_ioctl
Change since V3:
- Return EINVAL for _DRM_VBLANK_SIGNAL (Daniel)
Change since V2:
Daniel Vetter and Chris Wilson
- Replace ENOTTY by EOPNOTSUPP
- Return EINVAL if the parameters are wrong
Cc: Keith Packard <keithp@keithp.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191002140516.adeyj3htylimmlmg@smtp.gmail.com
DP 1.3 specification introduces the Link Training-tunable PHY Repeater,
and DP 1.4* supplemented it with new features. In the 1.4a spec, it was
introduced some innovations to make handy to add support for systems
with Thunderbolt or other repeater devices.
It is important to highlight that DP specification had some updates from
1.3 through 1.4a. In particular, DP 1.4 defines Repeater_FEC_CAPABILITY
at the address 0xf0004, and DP 1.4a redefined the address 0xf0004 to
DP_MAX_LANE_COUNT_PHY_REPEATER.
Changes since V4:
- Update commit message
- Fix misleading comments related to the spec version
Changes since V3:
- Replace spaces by tabs
Changes since V2:
- Drop the kernel-doc comment
- Reorder LTTPR according to register offset
Changes since V1:
- Adjusts registers names to be aligned with spec and the rest of the
file
- Update spec comment from 1.4 to 1.4a
Cc: Abdoulaye Berthe <Abdoulaye.Berthe@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: Abdoulaye Berthe <Abdoulaye.Berthe@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190909212144.deeomlsqihwg4l3y@outlook.office365.com
Configure the display Quality of service (QoS) levels priority if the
optional property node "arm,malidp-aqros-value" is defined in DTS file.
QoS signaling using AQROS and AWQOS AXI interface signals, the AQROS is
driven from the "RQOS" register, so needed to program the RQOS register
to avoid the high resolutions flicker issue on the LS1028A platform.
Signed-off-by: Wen He <wen.he_1@nxp.com>
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190910075913.17650-2-wen.he_1@nxp.com
The LCD timing definitions between Linux DRM vs Allwinner are different,
below diagram shows this clear differences.
Active Front Sync Back
Region Porch Porch
<-----------------------><----------------><--------------><-------------->
//////////////////////|
////////////////////// |
////////////////////// |.................. ................
________________
<----- [hv]display ----->
<------------- [hv]sync_start ------------>
<--------------------- [hv]sync_end ---------------------->
<-------------------------------- [hv]total ------------------------------>
<----- lcd_[xy] --------> <- lcd_[hv]spw ->
<---------- lcd_[hv]bp --------->
<-------------------------------- lcd_[hv]t ------------------------------>
The DSI driver misinterpreted the vbp term from the BSP code to refer
only to the backporch, when in fact it was backporch + sync. Thus the
driver incorrectly used the vertical front porch plus sync in its
calculation of the DRQ set bit value, when it should not have included
the sync timing.
Including additional sync timings leads to flip_done timed out as:
WARNING: CPU: 0 PID: 31 at drivers/gpu/drm/drm_atomic_helper.c:1429 drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0
[CRTC:46:crtc-0] vblank wait timed out
Modules linked in:
CPU: 0 PID: 31 Comm: kworker/0:1 Not tainted 5.1.0-next-20190514-00029-g09e5b0ed0a58 #18
Hardware name: Allwinner sun8i Family
Workqueue: events deferred_probe_work_func
[<c010ed54>] (unwind_backtrace) from [<c010b76c>] (show_stack+0x10/0x14)
[<c010b76c>] (show_stack) from [<c0688c70>] (dump_stack+0x84/0x98)
[<c0688c70>] (dump_stack) from [<c011d9e4>] (__warn+0xfc/0x114)
[<c011d9e4>] (__warn) from [<c011da40>] (warn_slowpath_fmt+0x44/0x68)
[<c011da40>] (warn_slowpath_fmt) from [<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1+0x298/0x2a0)
[<c040cd50>] (drm_atomic_helper_wait_for_vblanks.part.1) from [<c040e694>] (drm_atomic_helper_commit_tail_rpm+0x5c/0x6c)
[<c040e694>] (drm_atomic_helper_commit_tail_rpm) from [<c040e4dc>] (commit_tail+0x40/0x6c)
[<c040e4dc>] (commit_tail) from [<c040e5cc>] (drm_atomic_helper_commit+0xbc/0x128)
[<c040e5cc>] (drm_atomic_helper_commit) from [<c0411b64>] (restore_fbdev_mode_atomic+0x1cc/0x1dc)
[<c0411b64>] (restore_fbdev_mode_atomic) from [<c04156f8>] (drm_fb_helper_restore_fbdev_mode_unlocked+0x54/0xa0)
[<c04156f8>] (drm_fb_helper_restore_fbdev_mode_unlocked) from [<c0415774>] (drm_fb_helper_set_par+0x30/0x54)
[<c0415774>] (drm_fb_helper_set_par) from [<c03ad450>] (fbcon_init+0x560/0x5ac)
[<c03ad450>] (fbcon_init) from [<c03eb8a0>] (visual_init+0xbc/0x104)
[<c03eb8a0>] (visual_init) from [<c03ed1b8>] (do_bind_con_driver+0x1b0/0x390)
[<c03ed1b8>] (do_bind_con_driver) from [<c03ed780>] (do_take_over_console+0x13c/0x1c4)
[<c03ed780>] (do_take_over_console) from [<c03ad800>] (do_fbcon_takeover+0x74/0xcc)
[<c03ad800>] (do_fbcon_takeover) from [<c013c9c8>] (notifier_call_chain+0x44/0x84)
[<c013c9c8>] (notifier_call_chain) from [<c013cd20>] (__blocking_notifier_call_chain+0x48/0x60)
[<c013cd20>] (__blocking_notifier_call_chain) from [<c013cd50>] (blocking_notifier_call_chain+0x18/0x20)
[<c013cd50>] (blocking_notifier_call_chain) from [<c03a6e44>] (register_framebuffer+0x1e0/0x2f8)
[<c03a6e44>] (register_framebuffer) from [<c04153c0>] (__drm_fb_helper_initial_config_and_unlock+0x2fc/0x50c)
[<c04153c0>] (__drm_fb_helper_initial_config_and_unlock) from [<c04158c8>] (drm_fbdev_client_hotplug+0xe8/0x1b8)
[<c04158c8>] (drm_fbdev_client_hotplug) from [<c0415a20>] (drm_fbdev_generic_setup+0x88/0x118)
[<c0415a20>] (drm_fbdev_generic_setup) from [<c043f060>] (sun4i_drv_bind+0x128/0x160)
[<c043f060>] (sun4i_drv_bind) from [<c044b598>] (try_to_bring_up_master+0x164/0x1a0)
[<c044b598>] (try_to_bring_up_master) from [<c044b668>] (__component_add+0x94/0x140)
[<c044b668>] (__component_add) from [<c0445e1c>] (sun6i_dsi_probe+0x144/0x234)
[<c0445e1c>] (sun6i_dsi_probe) from [<c0452ef4>] (platform_drv_probe+0x48/0x9c)
[<c0452ef4>] (platform_drv_probe) from [<c04512cc>] (really_probe+0x1dc/0x2c8)
[<c04512cc>] (really_probe) from [<c0451518>] (driver_probe_device+0x60/0x160)
[<c0451518>] (driver_probe_device) from [<c044f7a4>] (bus_for_each_drv+0x74/0xb8)
[<c044f7a4>] (bus_for_each_drv) from [<c045107c>] (__device_attach+0xd0/0x13c)
[<c045107c>] (__device_attach) from [<c0450474>] (bus_probe_device+0x84/0x8c)
[<c0450474>] (bus_probe_device) from [<c0450900>] (deferred_probe_work_func+0x64/0x90)
[<c0450900>] (deferred_probe_work_func) from [<c0135970>] (process_one_work+0x204/0x420)
[<c0135970>] (process_one_work) from [<c013690c>] (worker_thread+0x274/0x5a0)
[<c013690c>] (worker_thread) from [<c013b3d8>] (kthread+0x11c/0x14c)
[<c013b3d8>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
Exception stack(0xde539fb0 to 0xde539ff8)
9fa0: 00000000 00000000 00000000 00000000
9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
---[ end trace 495200a78b24980e ]---
random: fast init done
[drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:46:crtc-0] flip_done timed out
[drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CONNECTOR:48:DSI-1] flip_done timed out
[drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:30:plane-0] flip_done timed out
With the terms(as described in above diagram) fixed, the panel
displays correctly without any timeouts.
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20191006160303.24413-2-icenowy@aosc.io
The DDC/CI protocol involves sending a multi-byte request to the
display via I2C, which is typically followed by a multi-byte
response. The internal I2C controller only allows single byte
reads/writes or reads of 8 sequential bytes, hence DDC/CI is not
supported when the internal I2C controller is used. The I2C
transfers complete without errors, however the data in the response
is garbage. Abort transfers to/from slave address 0x37 (DDC) with
-EOPNOTSUPP, to make it evident that the communication is failing.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Sean Paul <sean@poorly.run>
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191002124354.v2.1.I709dfec496f5f0b44a7b61dcd4937924da8d8382@changeid
It is possible for one HDMI connector to have multiple CEC adapters. The
typical real-world scenario is that where one adapter is used when the
device is in standby, and one that's better/smarter when the device is
powered up.
The cec-notifier changes were made with that in mind, but I missed that in
order to support this you need to tell cec_notifier_cec_adap_unregister()
which adapter you are unregistering from the notifier.
Add this additional argument. It is currently unused, but once all drivers
use this, the CEC core will be adapted for these use-cases.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/e9fc8740-6be6-43a7-beee-ce2d7b54936e@xs4all.nl
Add the DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED modifier to
denote the 16x16 block u-interleaved format used in Arm Utgard and
Midgard GPUs.
Changes from v1:-
1. Reserved the upper four bits (out of the 56 bits assigned to each vendor)
to denote the category of Arm specific modifiers. Currently, we have two
categories ie AFBC and MISC.
Changes from v2:-
1. Preserved Ray's authorship
2. Cleanups/changes suggested by Brian
3. Added r-bs of Brian and Qiang
Signed-off-by: Raymond Smith <raymond.smith@arm.com>
Reviewed-by: Brian Starkey <brian.starkey@arm.com>
Reviewed-by: Qiang Yu <yuq825@gmail.com>
Signed-off-by: Ayan kumar halder <ayan.halder@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191004141222.22337-1-ayan.halder@arm.com
