The A5XX GPU has really good hardware fault detection that can
detect a abnormal hardware condition and fire an interrupt in
a matter of milliseconds which is a lot better than waiting for
the hangcheck timer.
Enable the interrupt and log information before kicking off
recovery.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Commit eeb754746b ("drm/msm/gpu: use pm-runtime") adds a pointer
for the GPU platform device to the msm_gpu struct so we can
happily remove the same pointers from the individual GPU
structs.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
MDSS represents the top level wrapper that contains MDP5, DSI, HDMI and
other sub-blocks. W.r.t device heirarchy, it's the parent of all these
devices. The power domain of this device is actually tied to the GDSC
hw. When any sub-device enables its PD, MDSS's PD is also enabled.
The suspend/resume ops enable the top level clocks that end at the MDSS
boundary. For now, we're letting them all be optional, since the child
devices anyway hold a ref to these clocks.
Until now, we'd called a runtime_get() during probe, which ensured that
the GDSC was always on. Now that we've set up runtime PM for the children
devices, we can get rid of this hack.
Note: that the MDSS device is the platform_device in msm_drv.c. The
msm_runtime_suspend/resume ops call the funcs that enable/disable
the top level MDSS clocks. This is different from MDP4, where the
platform device created in msm_drv.c represents MDP4 itself. It would
have been nicer to hide these differences by adding new kms funcs, but
runtime PM needs to be enabled before kms is set up (i.e, msm_kms_init
is called).
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Requests for assigning/freeing SMP blocks by planes are collected during
the atomic check phase, and represented by mdp5_smp_state's 'assigned'
and 'released' members.
Once the atomic state is committed, these members are reset to 0,
indicating that the existing configuration satisfies all the planes.
Future atomic commits will copy the old mdp5_smp_state, and the 'assigned'
and 'released' members would be updated only if there was a change in
the plane configurations.
When we disable and re-enable display, we lose the values we wrote to the
SMP registers, but the code doesn't program the registers because there
isn't any change in mdp5_smp_state.
Fix this by writing to the registers irrespective of whether there was
a change in SMP state or not. We do this by keeping a cache of the
register values, and write them every time we commit a state.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
We shouldn't use use mode_set/mode_set_nofb helpers when we use runtime
PM. The registers configured in these funcs lose their state when we
eventually enable the display pipeline.
Do not implement these vfuncs in the helpers, and call them in the
crtc_enable/encoder_enable paths instead.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
The bus clocks are always enabled/disabled along with the power
domain, so move it to the runtime suspend/resume ops. This cleans
up the clock code a bit. Get rid of the clk_mutex mutex since it
isn't needed.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Call the pm_runtime_get/put API where we need the clocks enabled.
The main entry/exit points are 1) enabling/disabling the DSI bridge
and 2) Sending commands from the DSI host to the device.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Enable rudimentary runtime PM in the HDMI driver. We can't really do
agressive PM toggling at the moment because we need to leave the hpd
clocks enabled all the time. There isn't much benefit of creating
suspend/resume ops to toggle clocks either.
We just make sure that we configure the power domain in the HDMI bridge's
enable/disable paths, and the HDMI connector's detect() op.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
mdp5_enable/disable calls are scattered all around in the MDP5 code.
Use the pm_runtime_get/put calls here instead, and populate the
runtime PM suspend/resume ops to manage the clocks.
About the overall design: MDP5 is a child of the top level MDSS
device. MDSS is also the parent to DSI, HDMI and other interfaces. When
we enable MDP5's power domain, we end up enabling MDSS's PD too. It is
only MDSS's PD that actually controlls the GDSC HW. Therefore, calling
runtime_get/put on the MDP5 device is like just requesting a vote to
enable/disable the GDSC.
Functionally, replacing the clock enable/disable calls with the RPM API
can result in the power domain (GDSC) state being toggled if no other
child isn't powered on. This can result in the register context being lost.
We make sure (in future commits) that code paths don't end up configuring
registers and then later lose state, resulting in a bad HW state.
For now, we've replaced each mdp5_enable/disable with runtime_get/put API.
We could optimize things later by removing runtime_get/put calls which
don't really need to be there. This could prevent unnecessary toggling of
the power domain and clocks.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
In zap_shader_load_mdt(), we pass a pointer to a phys_addr_t
into dmam_alloc_coherent, which the compiler warns about:
drivers/gpu/drm/msm/adreno/a5xx_gpu.c: In function 'zap_shader_load_mdt':
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:54:50: error: passing argument 3 of 'dmam_alloc_coherent' from incompatible pointer type [-Werror=incompatible-pointer-types]
The returned DMA address is later passed on to a function that
takes a phys_addr_t, so it's clearly wrong to use the DMA
mapping interface here: the memory may be uncached, or the
address may be completely wrong if there is an IOMMU connected
to the device. What the code actually wants to do is to get
the physical address from the reserved-mem node. It goes through
the dma-mapping interfaces for obscure reasons, and this
apparently only works by chance, relying on specific bugs
in the error handling of the arm64 dma-mapping implementation.
The same problem existed in the "venus" media driver, which was
now fixed by Stanimir Varbanov after long discussions.
In order to make some progress here, I have now ported his
approach over to the adreno driver. The patch is currently
untested, and should get a good review, but it is now much
simpler than the original, and it should be obvious what
goes wrong if I made a mistake in the port.
See also: a6e2d36bf6 ("media: venus: don't abuse dma_alloc for non-DMA allocations")
Cc: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Fixes: 7c65817e6d ("drm/msm: gpu: Enable zap shader for A5XX")
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-and-Tested-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Rob Clark <robdclark@gmail.com>
When compile-testing for something other than ARCH_QCOM,
we run into a link error:
drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_hw_init':
a5xx_gpu.c:(.text.a5xx_hw_init+0x600): undefined reference to `qcom_mdt_get_size'
a5xx_gpu.c:(.text.a5xx_hw_init+0x93c): undefined reference to `qcom_mdt_load'
There is already an #ifdef that tries to check for CONFIG_QCOM_MDT_LOADER,
but that symbol is only meaningful when building for ARCH_QCOM.
This adds a compile-time check for ARCH_QCOM, and clarifies the
Kconfig select statement so we don't even try it for other targets.
The check for CONFIG_QCOM_MDT_LOADER can then go away, which also
improves compile-time coverage and makes the code a little nicer
to read.
Fixes: 7c65817e6d ("drm/msm: gpu: Enable zap shader for A5XX")
Acked-by: Jordan Crouse <jcrouse@codeaurora.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Rob Clark <robdclark@gmail.com>
msm_gpu's get_timestamp() op (called by the MSM_GET_PARAM ioctl) can
result in register accesses. We need our power domain and clocks to
be active for that. Make sure they are enabled here.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Fix a typo in msm_ioctl_gem_submit - check args->flags for the
MSM_SUBMIT_NO_IMPLICIT flag instead of args->fence.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
On A5XX GPU hardware clock gating needs to be turned off before
reading certain GPU registers via AHB. Turn off HWCG before calling
adreno_show() to safely dump all the registers without a system hang.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
There are some use cases wherein we need to turn off hardware clock
gating before reading certain registers. Modify the A5XX HWCG function
to allow user to enable or disable clock gating at will.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
The 0xf400 and 0xf800 ranges are in the RBBM_SECVID block which may
be protected from CPU access. Skip dumping them since they are minimally
useful for debugging and they aren't worth a system hang.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
We have upstream bindings (msm8916) that have the "_clk" suffix in the
clock names. The downstream bindings also require it.
We want to drop the "_clk" suffix and at the same time support existing
bindings. Update the MDP5 code with the the msm_clk_get() helper to
support both old and new clock names.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
The mdp5_cmd_encoder_disable is accidentally called in the encoder enable
path. We've not seen any problems since we haven't tested with command
mode panels in a while. Fix the copy-paste error.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
After the commit mentioned below, we start computing the byte and pixel
clocks (dsi_calc_clk_rate) in the DSI bridge's mode_set() op. The
calculation involves the number of DSI lanes being used by the
downstream bridge/panel.
If the downstream bridge/panel tries to change the number of DSI lanes
(as done in the ADV7533 driver) in its mode_set() op, then our DSI
host driver will not have the correct number of lanes when computing
byte/pixel clocks.
Fix this by delaying the clock rate calculation in the DSI bridge
enable path. In particular, compute the clock rates in
msm_dsi_host_get_phy_clk_req().
This fixes the DSI host error interrupts seen when we try to switch
between modes that require different number of lanes (4 to 3 lanes, or
vice versa) on db410c. The error interrupts occur since the byte/pixel
clock rates aren't according to what the DSI video mode timing engine
expects.
Fixes: b62aa70a98 ("drm/msm/dsi: Move PHY operations out of host")
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
We recently added locking to this function but there was a direct return
that was overlooked where we need to unlock.
Fixes: 0e08270a1f ("drm/msm: Separate locking of buffer resources from struct_mutex")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Rob Clark <robdclark@gmail.com>
We recently added an integer overflow check but it needs an additional
tweak to work properly on 32 bit systems.
The problem is that we're doing the right hand side of the assignment as
type unsigned long so the max it will have an integer overflow instead
of being larger than SIZE_MAX. That means the "sz > SIZE_MAX" condition
is never true even on 32 bit systems. We need to first cast it to u64
and then do the math.
Fixes: 4a630fadbb ("drm/msm: Fix potential buffer overflow issue")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Following compilation warnings were observed for these files:
CC [M] drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.o
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c: In function 'blend_setup':
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c:223:7: warning: missing braces around initializer [-Wmissing-braces]
enum mdp5_pipe stage[STAGE_MAX + 1][MAX_PIPE_STAGE] = { SSPP_NONE };
^
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c:223:7: warning: (near initialization for 'stage[0]') [-Wmissing-braces]
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c:224:7: warning: missing braces around initializer [-Wmissing-braces]
enum mdp5_pipe r_stage[STAGE_MAX + 1][MAX_PIPE_STAGE] = { SSPP_NONE };
^
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c:224:7: warning: (near initialization for 'r_stage[0]') [-Wmissing-braces]
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c: In function 'mdp5_plane_mode_set':
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:892:9: warning: missing braces around initializer [-Wmissing-braces]
struct phase_step step = { 0 };
^
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:892:9: warning: (near initialization for 'step.x') [-Wmissing-braces]
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:893:9: warning: missing braces around initializer [-Wmissing-braces]
struct pixel_ext pe = { 0 };
^
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c:893:9: warning: (near initialization for 'pe.left') [-Wmissing-braces]
This happens because in the first case we were initializing a two
dimensional array with {0} and in the second case we were initializing a
struct containing two arrays with {0}.
Fix them by adding another pair of {}.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
In function submit_create, if nr_cmds or nr_bos is assigned with
negative value, the allocated buffer may be small than intended.
Using this buffer will lead to buffer overflow issue.
Signed-off-by: Kasin Li <donglil@codeaurora.org>
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Buffer object specific resources like pages, domains, sg list
need not be protected with struct_mutex. They can be protected
with a buffer object level lock. This simplifies locking and
makes it easier to avoid potential recursive locking scenarios
for SVM involving mmap_sem and struct_mutex. This also removes
unnecessary serialization when creating buffer objects, and also
between buffer object creation and GPU command submission.
Signed-off-by: Sushmita Susheelendra <ssusheel@codeaurora.org>
[robclark: squash in handling new locking for shrinker]
Signed-off-by: Rob Clark <robdclark@gmail.com>
A 2 pixel wide pink strip was observed on the left end of some HDMI
monitors configured in a HDMI mode.
It turned out that we were missing out on configuring AVI infoframes, and
unlike APQ8064, the 8x96 HDMI H/W seems to be sensitive to that.
Add configuration of AVI infoframes. While at it, make sure that
hdmi_audio_update is only called when we've detected that the monitor
supports HDMI.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Without doing anything in unprepare, the HDMI driver isn't able to
switch modes successfully. Calling set_rate with a new rate results
in an un-locked PLL.
If we reset the PLL in unprepare, the PLL is able to lock with the
new rate.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Commit c0c0d9eeeb ("drm/msm: hdmi audio support") uses logical
OR operators to build up a value to be written in the
REG_HDMI_AUDIO_INFO0 and REG_HDMI_AUDIO_INFO1 registers when it
should have used bitwise operators.
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
Fixes: c0c0d9eeeb ("drm/msm: hdmi audio support")
Signed-off-by: Rob Clark <robdclark@gmail.com>
Now that the msm_gem supports an arbitrary number of vma's, we no longer
need to assign an id (index) to each address space. So rip out the
associated code.
Signed-off-by: Rob Clark <robdclark@gmail.com>
It means we have to do a list traversal where we once had an index into
a table. But the list will normally have one or two entries.
Signed-off-by: Rob Clark <robdclark@gmail.com>
Pull some of the logic out into msm_gem_new() (since we don't need to
care about the imported-bo case), and don't defer allocating pages. The
latter is generally a good idea, since if we are using VRAM carveout to
allocate contiguous buffers (ie. no IOMMU), the allocation is more
likely to fail. So failing at allocation time is a more sane option.
Plus this simplifies things in the next patch.
Signed-off-by: Rob Clark <robdclark@gmail.com>
No functional change, that will come later. But this will make it
easier to deal with dynamically created address spaces (ie. per-
process pagetables for gpu).
Signed-off-by: Rob Clark <robdclark@gmail.com>
Before we can shift to passing the address-space object to _get_iova(),
we need to fix a few places (dsi+fbdev) that were hard-coding the adress
space id. That gets somewhat easier if we just move these to the kms
base class.
Prep work for next patch.
Signed-off-by: Rob Clark <robdclark@gmail.com>
It serves no purpose, things should be sufficiently synchronized already
by atomic framework. And it is somewhat awkward to be holding a spinlock
when msm_gem_iova() is going to start needing to grab a mutex.
Signed-off-by: Rob Clark <robdclark@gmail.com>
Most, but not all, paths where calling the with struct_mutex held. The
fast-path in msm_gem_get_iova() (plus some sub-code-paths that only run
the first time) was masking this issue.
So lets just always hold struct_mutex for hw_init(). And sprinkle some
WARN_ON()'s and might_lock() to avoid this sort of problem in the
future.
Signed-off-by: Rob Clark <robdclark@gmail.com>
memptrs->wptr seems to be unused. Remove it to avoid
confusing the upcoming preemption code.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
The amount of information that we need to pass into msm_gpu_init()
is steadily increasing, so add a new struct to stabilize the function
call and make it easier to add new configuration down the line.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Modify the 'pad' member of struct drm_msm_gem_info to 'flags'. If the
user sets 'flags' to non-zero it means that they want a IOVA for the
GEM object instead of a mmap() offset. Return the iova in the 'offset'
member.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
[robclark: s/hint/flags in commit msg]
Signed-off-by: Rob Clark <robdclark@gmail.com>
There isn't any generic code that uses ->idle so remove it.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
The ioctl array is sparsely populated but the compiler will make sure
that it is sufficiently sized for all the values that we have so we
can safely use ARRAY_SIZE() instead of having a constantly changing
#define in the uapi header.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
The A5XX GPU powers on in "secure" mode. In secure mode the GPU can
only render to buffers that are marked as secure and inaccessible
to the kernel and user through a series of hardware protections. In
practice secure mode is used to draw things like a UI on a secure
video frame.
In order to switch out of secure mode the GPU executes a special
shader that clears out the GMEM and other sensitve registers and
then writes a register. Because the kernel can't be trusted the
shader binary is signed and verified and programmed by the
secure world. To do this we need to read the MDT header and the
segments from the firmware location and put them in memory and
present them for approval.
For targets without secure support there is an out: if the
secure world doesn't support secure then there are no hardware
protections and we can freely write the SECVID_TRUST register from
the CPU. We don't have 100% confidence that we can query the
secure capabilities at run time but we have enough calls that
need to go right to give us some confidence that we're at least doing
something useful.
Of course if we guess wrong you trigger a permissions violation
which usually ends up in a system crash but thats a problem
that shows up immediately.
[v2: use child device per Bjorn]
[v3: use generic MDT loader per Bjorn]
[v4: use managed dma functions and ifdefs for the MDT loader]
[v5: Add depends for QCOM_MDT_LOADER]
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
[robclark: fix Kconfig to use select instead of depends + #if IS_ENABLED()]
Signed-off-by: Rob Clark <robdclark@gmail.com>
-----BEGIN PGP SIGNATURE-----
iQEcBAABAgAGBQJZPdbLAAoJEHm+PkMAQRiGx4wH/1nCjfnl6fE8oJ24/1gEAOUh
biFdqJkYZmlLYHVtYfLm4Ueg4adJdg0wx6qM/4RaAzmQVvLfDV34bc1qBf1+P95G
kVF+osWyXrZo5cTwkwapHW/KNu4VJwAx2D1wrlxKDVG5AOrULH1pYOYGOpApEkZU
4N+q5+M0ce0GJpqtUZX+UnI33ygjdDbBxXoFKsr24B7eA0ouGbAJ7dC88WcaETL+
2/7tT01SvDMo0jBSV0WIqlgXwZ5gp3yPGnklC3F4159Yze6VFrzHMKS/UpPF8o8E
W9EbuzwxsKyXUifX2GY348L1f+47glen/1sedbuKnFhP6E9aqUQQJXvEO7ueQl4=
=m2Gx
-----END PGP SIGNATURE-----
BackMerge tag 'v4.12-rc5' into drm-next
Linux 4.12-rc5 for nouveau fixes
The overrun check for the size of submitted commands is off by one.
It should allow the offset plus the size to be equal to the
size of the memory object when the command stream is very tightly
constructed.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
Amongst its other duties, msm_gem_new_impl adds the newly created
GEM object to the shared inactive list which may also be actively
modifiying the list during submission. All the paths to modify
the list are protected by the mutex except for the one through
msm_gem_import which can end up causing list corruption.
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
[add extra WARN_ON(!mutex_is_locked(&dev->struct_mutex))]
Signed-off-by: Rob Clark <robdclark@gmail.com>
Use the dma_fence_match_context helper to check if all backing fences
are from our own context, in which case we don't have to wait.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Gustavo Padovan <gustavo.padovan@collabora.com>
[rebased on code-motion]
Signed-off-by: Rob Clark <robdclark@gmail.com>
struct irq_domain_ops is not modified, so it can be made const.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Signed-off-by: Rob Clark <robdclark@gmail.com>
