Lyude Paul [Fri, 11 Jan 2019 00:53:30 +0000 (19:53 -0500)]
drm/dp_mst: Restart last_connected_port_and_mstb() if topology ref fails
While this isn't a complete fix, this will improve the reliability of
drm_dp_get_last_connected_port_and_mstb() pretty significantly during
hotplug events, since there's a chance that the in-memory topology tree
may not be fully updated when drm_dp_get_last_connected_port_and_mstb()
is called and thus might end up causing our search to fail on an mstb
whose topology refcount has reached 0, but has not yet been removed from
it's parent.
Ideally, we should further fix this problem by ensuring that we deal
with the potential for racing with a hotplug event, which would look
like this:
* drm_dp_payload_send_msg() retrieves the last living relative of mstb
with drm_dp_get_last_connected_port_and_mstb()
* drm_dp_payload_send_msg() starts building payload message
At the same time, mstb gets unplugged from the topology and is no
longer the actual last living relative of the original mstb
* drm_dp_payload_send_msg() tries sending the payload message, hub times
out
* Hub timed out, we give up and run away-resulting in the payload being
leaked
This could be fixed by restarting the
drm_dp_get_last_connected_port_and_mstb() search whenever we get a
timeout, sending the payload to the new mstb, then repeating until
either the entire topology is removed from the system or
drm_dp_get_last_connected_port_and_mstb() fails. But since the above
race condition is not terribly likely, we'll address that in a later
patch series once we've improved the recovery handling for VCPI
allocations in the rest of the DP MST helpers.
Changes since v1:
* Convert kerneldoc for drm_dp_get_last_connected_port_and_mstb to
normal comment - danvet
Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Reviewed-by: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@redhat.com> Cc: Jerry Zuo <Jerry.Zuo@amd.com> Cc: Juston Li <juston.li@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-8-lyude@redhat.com
Lyude Paul [Fri, 11 Jan 2019 00:53:29 +0000 (19:53 -0500)]
drm/dp_mst: Introduce new refcounting scheme for mstbs and ports
The current way of handling refcounting in the DP MST helpers is really
confusing and probably just plain wrong because it's been hacked up many
times over the years without anyone actually going over the code and
seeing if things could be simplified.
To the best of my understanding, the current scheme works like this:
drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When
this refcount hits 0 for either of the two, they're removed from the
topology state, but not immediately freed. Both ports and branch devices
will reinitialize their kref once it's hit 0 before actually destroying
themselves. The intended purpose behind this is so that we can avoid
problems like not being able to free a remote payload that might still
be active, due to us having removed all of the port/branch device
structures in memory, as per:
commit 91a25e463130 ("drm/dp/mst: deallocate payload on port destruction")
Which may have worked, but then it caused use-after-free errors. Being
new to MST at the time, I tried fixing it;
commit 263efde31f97 ("drm/dp/mst: Get validated port ref in drm_dp_update_payload_part1()")
But, that was broken: both drm_dp_mst_port and drm_dp_mst_branch structs
are validated in almost every DP MST helper function. Simply put, this
means we go through the topology and try to see if the given
drm_dp_mst_branch or drm_dp_mst_port is still attached to something
before trying to use it in order to avoid dereferencing freed memory
(something that has happened a LOT in the past with this library).
Because of this it doesn't actually matter whether or not we keep keep
the ports and branches around in memory as that's not enough, because
any function that validates the branches and ports passed to it will
still reject them anyway since they're no longer in the topology
structure. So, use-after-free errors were fixed but payload deallocation
was completely broken.
Two years later, AMD informed me about this issue and I attempted to
come up with a temporary fix, pending a long-overdue cleanup of this
library:
commit c54c7374ff44 ("drm/dp_mst: Skip validating ports during destruction, just ref")
But then that introduced use-after-free errors, so I quickly reverted
it:
commit 9765635b3075 ("Revert "drm/dp_mst: Skip validating ports during destruction, just ref"")
And in the process, learned that there is just no simple fix for this:
the design is just broken. Unfortunately, the usage of these helpers are
quite broken as well. Some drivers like i915 have been smart enough to
avoid accessing any kind of information from MST port structures, but
others like nouveau have assumed, understandably so, that
drm_dp_mst_port structures are normal and can just be accessed at any
time without worrying about use-after-free errors.
After a lot of discussion, me and Daniel Vetter came up with a better
idea to replace all of this.
To summarize, since this is documented far more indepth in the
documentation this patch introduces, we make it so that drm_dp_mst_port
and drm_dp_mst_branch structures have two different classes of
refcounts: topology_kref, and malloc_kref. topology_kref corresponds to
the lifetime of the given drm_dp_mst_port or drm_dp_mst_branch in it's
given topology. Once it hits zero, any associated connectors are removed
and the branch or port can no longer be validated. malloc_kref
corresponds to the lifetime of the memory allocation for the actual
structure, and will always be non-zero so long as the topology_kref is
non-zero. This gives us a way to allow callers to hold onto port and
branch device structures past their topology lifetime, and dramatically
simplifies the lifetimes of both structures. This also finally fixes the
port deallocation problem, properly.
Additionally: since this now means that we can keep ports and branch
devices allocated in memory for however long we need, we no longer need
a significant amount of the port validation that we currently do.
Additionally, there is one last scenario that this fixes, which couldn't
have been fixed properly beforehand:
- CPU1 unrefs port from topology (refcount 1->0)
- CPU2 refs port in topology(refcount 0->1)
Since we now can guarantee memory safety for ports and branches
as-needed, we also can make our main reference counting functions fix
this problem by using kref_get_unless_zero() internally so that topology
refcounts can only ever reach 0 once.
Changes since v4:
* Change the kernel-figure summary for dp-mst/topology-figure-1.dot a
bit - danvet
* Remove figure numbers - danvet
Changes since v3:
* Remove rebase detritus - danvet
* Split out purely style changes into separate patches - hwentlan
Changes since v1:
* Remove forward declarations - danvet
* Move "Branch device and port refcounting" section from documentation
into kernel-doc comments - danvet
* Export internal topology lifetime functions into their own section in
the kernel-docs - danvet
* s/@/&/g for struct references in kernel-docs - danvet
* Drop the "when they are no longer being used" bits from the kernel
docs - danvet
* Modify diagrams to show how the DRM driver interacts with the topology
and payloads - danvet
* Make suggested documentation changes for
drm_dp_mst_topology_get_mstb() and drm_dp_mst_topology_get_port() -
danvet
* Better explain the relationship between malloc refs and topology krefs
in the documentation for drm_dp_mst_topology_get_port() and
drm_dp_mst_topology_get_mstb() - danvet
* Fix "See also" in drm_dp_mst_topology_get_mstb() - danvet
* Rename drm_dp_mst_topology_get_(port|mstb)() ->
drm_dp_mst_topology_try_get_(port|mstb)() and
drm_dp_mst_topology_ref_(port|mstb)() ->
drm_dp_mst_topology_get_(port|mstb)() - danvet
* s/should/must in docs - danvet
* WARN_ON(refcount == 0) in topology_get_(mstb|port) - danvet
* Move kdocs for mstb/port structs inline - danvet
* Split drm_dp_get_last_connected_port_and_mstb() changes into their own
commit - danvet
Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Reviewed-by: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@redhat.com> Cc: Jerry Zuo <Jerry.Zuo@amd.com> Cc: Juston Li <juston.li@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-7-lyude@redhat.com
This is a much more consistent naming scheme, and will make even more
sense once we redesign how the current refcounting scheme here works.
Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Daniel Vetter <daniel@ffwll.ch> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Cc: David Airlie <airlied@redhat.com> Cc: Jerry Zuo <Jerry.Zuo@amd.com> Cc: Juston Li <juston.li@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-6-lyude@redhat.com
Lyude Paul [Fri, 11 Jan 2019 00:53:27 +0000 (19:53 -0500)]
drm/dp_mst: Fix some formatting in drm_dp_mst_deallocate_vcpi()
Split some stuff across multiple lines
Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Reviewed-by: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@redhat.com> Cc: Jerry Zuo <Jerry.Zuo@amd.com> Cc: Juston Li <juston.li@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-5-lyude@redhat.com
Lyude Paul [Fri, 11 Jan 2019 00:53:26 +0000 (19:53 -0500)]
drm/dp_mst: Fix some formatting in drm_dp_mst_allocate_vcpi()
Fix some indenting, split some stuff across multiple lines.
Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Reviewed-by: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@redhat.com> Cc: Jerry Zuo <Jerry.Zuo@amd.com> Cc: Juston Li <juston.li@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-4-lyude@redhat.com
Lyude Paul [Fri, 11 Jan 2019 00:53:25 +0000 (19:53 -0500)]
drm/dp_mst: Fix some formatting in drm_dp_payload_send_msg()
Split some stuff across multiple lines, remove some unnecessary braces
Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Reviewed-by: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@redhat.com> Cc: Jerry Zuo <Jerry.Zuo@amd.com> Cc: Juston Li <juston.li@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-3-lyude@redhat.com
Lyude Paul [Fri, 11 Jan 2019 00:53:24 +0000 (19:53 -0500)]
drm/dp_mst: Fix some formatting in drm_dp_add_port()
Reindent some stuff, and split some stuff across multiple lines so we
aren't going over the text width limit.
Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Reviewed-by: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@redhat.com> Cc: Jerry Zuo <Jerry.Zuo@amd.com> Cc: Juston Li <juston.li@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-2-lyude@redhat.com
Ezequiel Garcia [Wed, 9 Jan 2019 18:56:38 +0000 (15:56 -0300)]
drm/rockchip: Separate RK3288 from RK3368 win01 registers
This commit splits the registers for RK3288 from those
for RK3328, RK3368 and RK3399. It seems RK3288 does not
support plane x-y-mirroring, and so in order to support this
for the other SoCs, we need to have separate set of registers
for win0 and win1.
Ezequiel Garcia [Wed, 9 Jan 2019 18:56:37 +0000 (15:56 -0300)]
drm/rockchip: Fix typo in VOP macros argument
Fix a small typo in the macros VOP argument. The macro argument
is currently wrongly named "x", and then never used. The code
built fine almost by accident, as the macros are always used
in a context where a proper "vop" symbol exists.
This fix is almost cosmetic, as the resulting code shouldn't change.
Currently, YUV hardware overlays are converted to RGB using
a color space conversion different than BT.601.
The result is that colors of e.g. NV12 buffers don't match
colors of YUV hardware overlays.
In order to fix this, enable YUV2YUV and set appropriate coefficients
for formats such as NV12 to be displayed correctly.
This commit was tested using modetest, gstreamer and chromeos (hardware
accelerated video playback). Before the commit, tests rendering
with NV12 format resulted in colors not displayed correctly.
Test examples (Tested on RK3399 and RK3288 boards
connected to HDMI monitor):
Linus Walleij [Thu, 10 Jan 2019 11:40:49 +0000 (12:40 +0100)]
drm/fb-helper: Scale back depth to supported maximum
The following happened when migrating an old fbdev driver to DRM:
The Integrator/CP PL111 supports 16BPP but only ARGB1555/ABGR1555
or XRGB1555/XBGR1555 i.e. the maximum depth is 15.
This makes the initialization of the framebuffer fail since
the code in drm_fb_helper_single_fb_probe() assigns the same value
to sizes.surface_bpp and sizes.surface_depth. I.e. it simply assumes
a 1-to-1 mapping between BPP and depth, which is true in most cases
but not for this hardware that only support odd formats.
To support the odd case of a driver supporting 16BPP with only 15
bits of depth, this patch will make the code loop over the formats
supported on the primary plane on each CRTC managed by the FB
helper and cap the depth to the maximum supported on any primary
plane.
On the PL110 Integrator, this makes drm_mode_legacy_fb_format()
select DRM_FORMAT_XRGB1555 which is acceptable for this driver, and
thus we get framebuffer, penguin and console on the Integrator/CP.
Ville Syrjälä [Tue, 8 Jan 2019 17:28:27 +0000 (19:28 +0200)]
drm/radeon: Use drm_hdmi_avi_infoframe_quant_range()
Fill out the AVI infoframe quantization range bits using
drm_hdmi_avi_infoframe_quant_range() instead of hand rolling it.
This changes the behaviour slightly as
drm_hdmi_avi_infoframe_quant_range() will set a non-zero Q bit
even when QS==0 iff the Q bit matched the default quantization
range for the given mode. This matches the recommendation in
HDMI 2.0 and is allowed even before that.
Cc: Alex Deucher <alexander.deucher@amd.com> Cc: "Christian König" <christian.koenig@amd.com> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com> Cc: amd-gfx@lists.freedesktop.org Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Acked-by: Alex Deucher <alexander.deucher@amd.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190108172828.15184-3-ville.syrjala@linux.intel.com
Ville Syrjälä [Tue, 8 Jan 2019 17:28:26 +0000 (19:28 +0200)]
drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well
Fill out the AVI infoframe quantization range bits using
drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as well.
This changes the behaviour slightly as
drm_hdmi_avi_infoframe_quant_range() will set a non-zero Q bit
even when QS==0 iff the Q bit matched the default quantization
range for the given mode. This matches the recommendation in
HDMI 2.0 and is allowed even before that.
Ville Syrjälä [Tue, 8 Jan 2019 17:28:25 +0000 (19:28 +0200)]
drm/edid: Pass connector to AVI infoframe functions
Make life easier for drivers by simply passing the connector
to drm_hdmi_avi_infoframe_from_display_mode() and
drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
need to worry about is_hdmi2_sink mess.
v2: Make is_hdmi2_sink() return true for sil-sii8620
Adapt to omap/vc4 changes
Cc: Alex Deucher <alexander.deucher@amd.com> Cc: "Christian König" <christian.koenig@amd.com> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com> Cc: Archit Taneja <architt@codeaurora.org> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Inki Dae <inki.dae@samsung.com> Cc: Joonyoung Shim <jy0922.shim@samsung.com> Cc: Seung-Woo Kim <sw0312.kim@samsung.com> Cc: Kyungmin Park <kyungmin.park@samsung.com> Cc: Russell King <linux@armlinux.org.uk> Cc: CK Hu <ck.hu@mediatek.com> Cc: Philipp Zabel <p.zabel@pengutronix.de> Cc: Rob Clark <robdclark@gmail.com> Cc: Ben Skeggs <bskeggs@redhat.com> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> Cc: Sandy Huang <hjc@rock-chips.com> Cc: "Heiko Stübner" <heiko@sntech.de> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org> Cc: Vincent Abriou <vincent.abriou@st.com> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Eric Anholt <eric@anholt.net> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Ilia Mirkin <imirkin@alum.mit.edu> Cc: amd-gfx@lists.freedesktop.org Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Cc: nouveau@lists.freedesktop.org Cc: linux-tegra@vger.kernel.org Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Acked-by: Thierry Reding <treding@nvidia.com> Acked-by: Russell King <rmk+kernel@armlinux.org.uk> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190108172828.15184-1-ville.syrjala@linux.intel.com
Daniel Vetter [Thu, 10 Jan 2019 10:30:45 +0000 (11:30 +0100)]
drm/crtc-helpers: WARN when used with atomic drivers
Motivated by an oversight of mine when looking at the atomic bochs
conversion. For consistency also switch over to the same style as used
elsewhere (e.g. in drm_mode_set_config_internal).
Sam Ravnborg [Tue, 8 Jan 2019 19:29:39 +0000 (20:29 +0100)]
drm: remove drmP.h from drm_gem_cma_helper.h
With all dependencies fixed we can now remove
drmP.h from drm_gem_cma_helper.h.
It is replaced by the include files required,
or forward declarations as appropritate.
Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Acked-by: Noralf Trønnes <noralf@tronnes.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <maxime.ripard@bootlin.com> Cc: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-13-sam@ravnborg.org
Sam Ravnborg [Tue, 8 Jan 2019 19:29:38 +0000 (20:29 +0100)]
drm/tinydrm: do not reply on drmP.h from drm_gem_cma_helper.h
drmP.h was the only header file in the past and a lot
of files rely on that drmP.h defines everything.
The goal is to one day to delete drmP.h and
as a step towards this it will no longer be included in the
headers files in include/drm/
To prepare tinydrm/ for this add dependencies that
othwewise was pulled in by drmP.h from drm_gem_cma_helper.h
To avoid that tinydrm.h became "include everything",
push include files to the individual drivers.
Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Acked-by: Noralf Trønnes <noralf@tronnes.org> Acked-by: David Lechner <david@lechnology.com> Cc: David Airlie <airlied@linux.ie> Cc: Eric Anholt <eric@anholt.net> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-12-sam@ravnborg.org
Sam Ravnborg [Tue, 8 Jan 2019 19:29:36 +0000 (20:29 +0100)]
drm/arc: do not rely on drmP.h from drm_gem_cma_helper.h
drmP.h was the only header file in the past and a lot
of files rely on that drmP.h defines everything.
The goal is to one day to delete drmP.h and
as a step towards this it will no longer be included in the
headers files in include/drm/
To prepare arc/ for this add dependencies that
othwewise was pulled in by drmP.h from drm_gem_cma_helper.h
Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Acked-by: Noralf Trønnes <noralf@tronnes.org> Cc: Alexey Brodkin <abrodkin@synopsys.com> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: David Airlie <airlied@linux.ie>
[danvet: Fix typo in commit message.] Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-10-sam@ravnborg.org
Sam Ravnborg [Tue, 8 Jan 2019 19:29:34 +0000 (20:29 +0100)]
drm: remove include of drmP.h from drm_encoder_slave.h
No further changes required.
Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Acked-by: Noralf Trønnes <noralf@tronnes.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <maxime.ripard@bootlin.com> Cc: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-8-sam@ravnborg.org
Sam Ravnborg [Tue, 8 Jan 2019 19:29:33 +0000 (20:29 +0100)]
drm: remove include of drmP.h from bridge/dw_hdmi.h
drmP.h is an relic from the days when there was a single header file.
To enable the removal of drmP.h from all users drop include
of drmP.h from bridge/dw_hdmi.h.
A few files relied on the file included in drmP.h - add explicit
include statements or forward declarations to these files.
Build tested with arm and x86.
v2:
- prefer forward declarations when possible (Laurent Pinchart)
- sort include files (Laurent Pinchart)
Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Archit Taneja <architt@codeaurora.org> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Cc: Fabio Estevam <fabio.estevam@nxp.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Maxime Ripard <maxime.ripard@bootlin.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-7-sam@ravnborg.org
Kuo-Hsin Yang [Tue, 8 Jan 2019 07:45:17 +0000 (15:45 +0800)]
drm/gem: Mark pinned pages as unevictable
The gem drivers use shmemfs to allocate backing storage for gem objects.
On Samsung Chromebook Plus, the drm/rockchip driver may call
rockchip_gem_get_pages -> drm_gem_get_pages -> shmem_read_mapping_page
to pin a lot of pages, breaking the page reclaim mechanism and causing
oom-killer invocation.
E.g. when the size of a zone is 3.9 GiB, the inactive_ratio is 5. If
active_anon / inactive_anon < 5 and all pages in the inactive_anon lru
are pinned, page reclaim would keep scanning inactive_anon lru without
reclaiming memory. It breaks page reclaim when the rockchip driver only
pins about 1/6 of the anon lru pages.
Mark these pinned pages as unevictable to avoid the premature oom-killer
invocation. See also similar patch on i915 driver [1].
Sam Ravnborg [Tue, 8 Jan 2019 19:29:30 +0000 (20:29 +0100)]
drm: make drm_framebuffer.h self contained
Add forward declaration and pull in include
file to make drm_framebuffer.h self contained.
While add it order include files alphabetically.
The use of TASK_COMM_LEN is the reason for including sched.h.
I could not see any good way to avoid this dependency,
and users of drm_framebuffer.comm already use
TASK_COMM_LEN to check for length etc.
v2:
- Added forward declaration of drm_gem_object (Noralf)
- Added ack from Noralf
Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Acked-by: Noralf Trønnes <noralf@tronnes.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <maxime.ripard@bootlin.com> Cc: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-4-sam@ravnborg.org
Sam Ravnborg [Tue, 8 Jan 2019 19:29:29 +0000 (20:29 +0100)]
drm: move DRM_SWITCH_POWER defines to drm_device.h
Move DRM_SWITCH_POWER out of drmP.h to allow users
to get rid of the drmP include.
Moved to drm_device.h because drm_device.switch_power_state
is the only user.
Converted to enum and added sparse kerneldoc comments.
Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <maxime.ripard@bootlin.com> Cc: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-3-sam@ravnborg.org
Sam Ravnborg [Tue, 8 Jan 2019 19:29:28 +0000 (20:29 +0100)]
drm: drm_device.h: update comments to kernel-doc style
Updated comment style to kernel-doc format in drm_device.h
In struct drm_device there are 12 struct members without doc:
- registered
- filelist_mutex
- filelist
- irq
- vbl_lock
- event_lock
- hose
- sigdata
- sigdata.context
- sigdata.lock
- agp_buffer_map
- agp_buffer_token
They all need proper documentation, a task left for someone
that knows their usage.
drm_device is not plugged into Documentation/gpu/drm-internals.rst
as this would create a new load of warnings.
Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <maxime.ripard@bootlin.com> Cc: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Jonathan Corbet <corbet@lwn.net> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-2-sam@ravnborg.org
Shayenne Moura [Thu, 20 Dec 2018 12:27:57 +0000 (10:27 -0200)]
drm: i915: Cleanup drm_display_mode print str
This patch adjust the print string of drm_display_mode object
to remove drm_mode_object dependency in i915 files.
It modifies the print style to standardize the use of DRM_MODE_FMT.
One of the more common cases of allocation size calculations is finding the
size of a structure that has a zero-sized array at the end, along with memory
for some number of elements for that array. For example:
Ezequiel Garcia [Tue, 8 Jan 2019 14:59:30 +0000 (11:59 -0300)]
drm/virtio: Drop deprecated load/unload initialization
Move the code around so the driver is probed the bus
.probe and removed from the bus .remove callbacks.
This commit is just a cleanup and shouldn't affect
functionality.
Peter Wu [Sun, 23 Dec 2018 00:55:07 +0000 (01:55 +0100)]
drm/fb-helper: fix leaks in error path of drm_fb_helper_fbdev_setup
After drm_fb_helper_fbdev_setup calls drm_fb_helper_init,
"dev->fb_helper" will be initialized (and thus drm_fb_helper_fini will
have some effect). After that, drm_fb_helper_initial_config is called
which may call the "fb_probe" driver callback.
This driver callback may call drm_fb_helper_defio_init (as is done by
drm_fb_helper_generic_probe) or set a framebuffer (as is done by bochs)
as documented. These are normally cleaned up on exit by
drm_fb_helper_fbdev_teardown which also calls drm_fb_helper_fini.
If an error occurs after "fb_probe", but before setup is complete, then
calling just drm_fb_helper_fini will leak resources. This was triggered
by df2052cc922 ("bochs: convert to drm_fb_helper_fbdev_setup/teardown"):
[ 50.008030] bochsdrmfb: enable CONFIG_FB_LITTLE_ENDIAN to support this framebuffer
[ 50.009436] bochs-drm 0000:00:02.0: [drm:drm_fb_helper_fbdev_setup] *ERROR* fbdev: Failed to set configuration (ret=-38)
[ 50.011456] [drm] Initialized bochs-drm 1.0.0 20130925 for 0000:00:02.0 on minor 2
[ 50.013604] WARNING: CPU: 1 PID: 1 at drivers/gpu/drm/drm_mode_config.c:477 drm_mode_config_cleanup+0x280/0x2a0
[ 50.016175] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G T 4.20.0-rc7 #1
[ 50.017732] EIP: drm_mode_config_cleanup+0x280/0x2a0
...
[ 50.023155] Call Trace:
[ 50.023155] ? bochs_kms_fini+0x1e/0x30
[ 50.023155] ? bochs_unload+0x18/0x40
This can be reproduced with QEMU and CONFIG_FB_LITTLE_ENDIAN=n.
Noralf Trønnes [Sat, 5 Jan 2019 18:18:46 +0000 (19:18 +0100)]
drm/fb-helper: generic: Fix setup error path
If register_framebuffer() fails during fbdev setup we will leak the
framebuffer, the GEM buffer and the shadow buffer for defio. This is
because drm_fb_helper_fbdev_setup() just calls drm_fb_helper_fini() on
error not taking into account that register_framebuffer() can fail.
Since the generic emulation uses DRM client for its framebuffer and
backing buffer in addition to a shadow buffer, it's necessary to open code
drm_fb_helper_fbdev_setup() to properly handle the error path.
Error cleanup is removed from .fb_probe and is handled by one function for
all paths.
Ezequiel Garcia [Wed, 2 Jan 2019 17:55:06 +0000 (14:55 -0300)]
drm/virtio: Remove incorrect kfree()
The virtio_gpu_output is a member of struct virtio_gpu_device
and is not a dynamically-allocated chunk, so it's wrong to kfree() it.
Removing it fixes a memory corruption BUG() that can be triggered
when the virtio-gpu driver is removed.
Chris Wilson [Sun, 30 Dec 2018 12:28:42 +0000 (12:28 +0000)]
drm: Reorder set_property_atomic to avoid returning with an active ww_ctx
Delay the drm_modeset_acquire_init() until after we check for an
allocation failure so that we can return immediately upon error without
having to unwind.
WARNING: lock held when returning to user space!
4.20.0+ #174 Not tainted
------------------------------------------------
syz-executor556/8153 is leaving the kernel with locks still held!
1 lock held by syz-executor556/8153:
#0: 000000005100c85c (crtc_ww_class_acquire){+.+.}, at:
set_property_atomic+0xb3/0x330 drivers/gpu/drm/drm_mode_object.c:462
Reported-by: syzbot+6ea337c427f5083ebdf2@syzkaller.appspotmail.com Fixes: 144a7999d633 ("drm: Handle properties in the core for atomic drivers") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie> Cc: <stable@vger.kernel.org> # v4.14+ Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20181230122842.21917-1-chris@chris-wilson.co.uk
Jani Nikula [Fri, 28 Dec 2018 08:28:15 +0000 (10:28 +0200)]
drm: forward declare struct drm_file in drm_syncobj.h
drm_syncobj.h uses struct drm_file pointers, forward declare struct
drm_file to make the header self-contained. This prepares for dropping
drmP.h from files including drm_syncobj.h.
Switch from "" to <> includes while at it.
v2: forward declare instead of including drm_file.h (Daniel)
[Updated commit message per Laurent's review while applying.]
Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20181228082815.10797-1-jani.nikula@intel.com
Jani Nikula [Thu, 27 Dec 2018 12:56:39 +0000 (14:56 +0200)]
drm: include types.h from drm_hdcp.h
drm_hdcp.h uses kernel integer types, make it self-contained by
including linux/kernel.h. This prepares for dropping drmP.h from files
including drm_hdcp.h.
[Updated commit message per Laurent's review while applying.]
Jani Nikula [Thu, 27 Dec 2018 12:56:38 +0000 (14:56 +0200)]
drm: include idr.h from drm_file.h
drm_file.h embeds idr structures in DRM-specific structures. Include the
corresponding header to make drm_file.h self-contained. Make it easier
to drop drmP.h includes.
[Updated commit message per Laurent's review while applying.]
Jani Nikula [Thu, 27 Dec 2018 12:56:37 +0000 (14:56 +0200)]
drm: include kernel.h and agp_backend.h from intel-gtt.h
intel-gtt.h uses kernel and agp pointers, make it self-contained by
including the relevant headers. This prepares for dropping drmP.h from
files including intel-gtt.h.
[Updated commit message per Laurent's review while applying.]
Sam Ravnborg [Wed, 26 Dec 2018 21:03:47 +0000 (22:03 +0100)]
drm: move DRM_IF_VERSION to drm_internal.h
Move DRM_IF_VERSION out of drmP.h to allow users
to get rid of the drmP include.
Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <maxime.ripard@bootlin.com> Cc: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20181226210353.13993-1-sam@ravnborg.org
Gerd Hoffmann [Thu, 20 Dec 2018 10:11:21 +0000 (11:11 +0100)]
drm/bochs: add edid present check
Check header before trying to read the complete edid blob, to avoid the
log being spammed in case qemu has no edid support (old qemu or edid
support turned off).
Boris Brezillon [Thu, 6 Dec 2018 14:24:38 +0000 (15:24 +0100)]
drm/vc4: Take margin setup into account when updating planes
Applyin margins is just a matter of scaling all planes appropriately
and adjusting the CRTC X/Y offset to account for the
left/right/top/bottom borders.
Create a vc4_plane_margins_adj() function doing that and call it from
vc4_plane_setup_clipping_and_scaling() so that we are ready to attach
margins properties to the HDMI connector.
Boris Brezillon [Thu, 6 Dec 2018 14:24:37 +0000 (15:24 +0100)]
drm/connector: Allow creation of margin props alone
TV margins properties can only be added as part of the SDTV TV
connector properties creation, but we might need those props for HDMI
TVs too, so let's move the margins props creation in a separate
function and expose it to drivers.
We also add an helper to attach margins props to a connector.
MAINTAINERS: drm: Remove myself as drm-bridge maintainer
I have moved on to other stuff for now. Haven't been able to make time
to review bridge related work. Andrzej has been doing it by himself
for a while now.
Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Gustavo Padovan <gustavo@padovan.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Sean Paul <sean@poorly.run> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Archit Taneja <architt@codeaurora.org> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180913075300.22510-1-architt@codeaurora.org
Dan Carpenter [Mon, 17 Dec 2018 07:03:44 +0000 (10:03 +0300)]
drm/ati_pcigart: Fix error code in drm_ati_pcigart_init()
The drm_ati_pcigart_init() function was originally suppose to return one
on success and zero on failure, but these days it returns a mix of
zero, one and -ENOMEM on failure.
This patch cleans it up and modifies the caller so now the function
returns zero on success and negative error codes on failure.
Dan Carpenter [Mon, 17 Dec 2018 07:00:38 +0000 (10:00 +0300)]
drm: Fix an error pointer dereference()
The drm_mode_create_tile_group() is only called from
drm_parse_tiled_block() and the caller expects it to return a NULL on
error. In other words, this function should match
drm_mode_get_tile_group().
- Adds local variables in the first loop, instead of using array indices
everywhere
- Adds an early continue to reduce the indent level in the second loop
Lyude Paul [Tue, 11 Dec 2018 23:50:26 +0000 (18:50 -0500)]
drm/dp_mst: Fix memory leak in drm_dp_mst_topology_mgr_destroy()
We need to call drm_dp_mst_topology_mgr_set_mst(mgr, false) when
destroying the topology manager in order to ensure that the root mstb
and all of it's descendents are actually destroyed, and additionally to
try to make sure that we leave the hub in a clean state.
drm: Rename crtc_idr as object_idr to KMS cleanups
This patch solves this TODO task:
drm_mode_config.crtc_idr is misnamed, since it contains all KMS object.
Should be renamed to drm_mode_config.object_idr.
Heiko Stuebner [Fri, 30 Nov 2018 10:24:49 +0000 (11:24 +0100)]
drm/rockchip: Add implicit fencing support for planes
Render like lima will attach a fence to the framebuffer dma_buf,
so the display driver should wait for it to finish before showing
the framebufferto prevent tearing.
Generally tested on rk3188, rk3288, rk3328 and rk3399 and
together with an actual lima-based kmscube on rk3188 and rk3328.
Christian König [Fri, 7 Dec 2018 19:10:17 +0000 (20:10 +0100)]
drm/etnaviv: fix for 64bit seqno change
The fence seqno is now 64bit, fixes build warning.
Signed-off-by: Christian König <christian.koenig@amd.com> Acked-by: Lucas Stach <l.stach@pengutronix.de> Acked-by: Alex Deucher <alexander.deucher@amd.com> Link: https://patchwork.freedesktop.org/patch/267136/
Ville Syrjälä [Fri, 28 Sep 2018 18:04:00 +0000 (21:04 +0300)]
drm/dp/mst: Validate REMOTE_I2C_READ harder
Make sure i2c msgs we're asked to transfer conform to the
requirements of REMOTE_I2C_READ. We were only checking that the
last message is a read, but we must also check that the preceding
messages are all writes. Also check that the length of each
message isn't too long.
Ville Syrjälä [Fri, 28 Sep 2018 18:03:59 +0000 (21:03 +0300)]
drm/dp/mst: Configure no_stop_bit correctly for remote i2c xfers
We aren't supposed to force a stop+start between every i2c msg
when performing multi message transfers. This should eg. cause
the DDC segment address to be reset back to 0 between writing
the segment address and reading the actual EDID extension block.
To quote the E-DDC spec:
"... this standard requires that the segment pointer be
reset to 00h when a NO ACK or a STOP condition is received."
Since we're going to touch this might as well consult the
I2C_M_STOP flag to determine whether we want to force the stop
or not.
YueHaibing [Sat, 8 Dec 2018 08:36:25 +0000 (08:36 +0000)]
drm/ast: Remove set but not used variable 'bo'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/gpu/drm/ast/ast_fb.c: In function 'astfb_create':
drivers/gpu/drm/ast/ast_fb.c:194:17: warning:
variable 'bo' set but not used [-Wunused-but-set-variable]
It never used since introduction in commit 312fec1405dd ("drm: Initial KMS
driver for AST (ASpeed Technologies) 2000 series (v2)")
Boris Brezillon [Fri, 7 Dec 2018 08:36:06 +0000 (09:36 +0100)]
drm/vc4: Add support for X/Y reflection
Add support for X/Y reflection when the plane is using linear or
T-tiled formats. X/Y reflection hasn't been tested on SAND formats, so
we reject them until proper testing/debugging has been done.
Boris Brezillon [Fri, 7 Dec 2018 08:36:05 +0000 (09:36 +0100)]
drm/vc4: Fix negative X/Y positioning on SAND planes
Commit 3e407417b192 ("drm/vc4: Fix X/Y positioning of planes using
T_TILES modifier") fixed the problem with T_TILES format, but left
things in a non-working state for SAND formats. Address that now.
Eric Anholt [Mon, 3 Dec 2018 22:24:36 +0000 (14:24 -0800)]
drm/v3d: Drop the wait for L2T flush to complete.
According to Dave, once you've started an L2T flush, all L2T accesses
will be blocked until the flush completes. This fixes a consistent
3-4ms stall between the ioctl and running the job, and 3DMMES Taiji
goes from 27fps to 110fps.
v2: Leave a note about why we don't need to wait for completion.
Eric Anholt [Mon, 3 Dec 2018 22:24:35 +0000 (14:24 -0800)]
drm/v3d: Don't bother flushing L1TD at job start.
This is the write combiner for TMU writes. You're supposed to flush
that at job end if you had dirtied any cachelines. Flushing it at job
start then doesn't make any sense.
Mika Kuoppala [Fri, 7 Dec 2018 12:34:28 +0000 (14:34 +0200)]
drm/i915: Compile fix for 64b dma-fence seqno
Many errs of the form:
drivers/gpu/drm/i915/selftests/intel_hangcheck.c: In function ‘__igt_reset_evict_vma’:
./include/linux/kern_levels.h:5:18: error: format ‘%x’ expects argument of type ‘unsigned int’, but argum
Fixes: b312d8ca3a7c ("dma-buf: make fence sequence numbers 64 bit v2") Cc: Christian König <christian.koenig@amd.com> Cc: Chunming Zhou <david1.zhou@amd.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20181207123428.16257-1-mika.kuoppala@linux.intel.com
Christian König [Wed, 14 Nov 2018 15:11:06 +0000 (16:11 +0100)]
dma-buf: make fence sequence numbers 64 bit v2
For a lot of use cases we need 64bit sequence numbers. Currently drivers
overload the dma_fence structure to store the additional bits.
Stop doing that and make the sequence number in the dma_fence always
64bit.
For compatibility with hardware which can do only 32bit sequences the
comparisons in __dma_fence_is_later only takes the lower 32bits as significant
when the upper 32bits are all zero.
Christian König [Wed, 5 Dec 2018 19:43:02 +0000 (20:43 +0100)]
drm/v3d: fix broken build
I missed one case during the recent revert of the replace_fence
interface change.
Fixes: 0b258ed1a219 drm: revert "expand replace_fence to support timeline point v2" Signed-off-by: Christian König <christian.koenig@amd.com> Acked-by: Alex Deucher <alexander.deucher@amd.com> Link: https://patchwork.freedesktop.org/patch/266134/
The whole interface isn't thought through. Since this function can't
fail we actually can't allocate an object to store the sync point.
Sorry, I should have taken the lead on this from the very beginning and
reviewed it more thoughtfully. Going to propose a new interface as a
follow up change.
YueHaibing [Sat, 1 Dec 2018 03:20:44 +0000 (03:20 +0000)]
drm/vkms: Remove set but not used variable 'vkms_obj'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/gpu/drm/vkms/vkms_plane.c: In function 'vkms_prepare_fb':
drivers/gpu/drm/vkms/vkms_plane.c:144:26: warning:
variable 'vkms_obj' set but not used [-Wunused-but-set-variable]
It never used since introduction in commit 8ce1bb0b5337 ("drm/vkms: map/unmap buffers in [prepare/cleanup]_fb hooks")
Eric Anholt [Sat, 1 Dec 2018 00:57:58 +0000 (16:57 -0800)]
drm/v3d: Add more tracepoints for V3D GPU rendering.
The core scheduler tells us when the job is pushed to the scheduler's
queue, and I had the job_run functions saying when they actually queue
the job to the hardware. By adding tracepoints for the very top of
the ioctls and the IRQs signaling job completion, "perf record -a -e
v3d:.\* -e gpu_scheduler:.\* <job>; perf script" gets you a pretty
decent timeline.
Christian König [Tue, 13 Nov 2018 13:14:00 +0000 (14:14 +0100)]
drm/syncobj: use dma_fence_get_stub
Extract of useful code from the timeline work. Let's use just a single
stub fence instance instead of allocating a new one all the time.
Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chunming Zhou <david1.zhou@amd.com> Link: https://patchwork.freedesktop.org/patch/265248/
Christian König [Mon, 3 Dec 2018 12:36:14 +0000 (13:36 +0100)]
dma-buf: add dma_fence_get_stub
Extract of useful code from the timeline work. This provides a function
to return a stub or dummy fence which is always signaled.
Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chunming Zhou <david1.zhou@amd.com> Link: https://patchwork.freedesktop.org/patch/265248/