| 78f5685f | 21-Oct-2024 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Rename 'wait_lock' to 'irq_lock'
The 'wait_lock' name seems to be a copy-paste from omapdrm, and makes no
sense here. Rename it to 'irq_lock'. Also clarify the related comment to
make it
drm/tidss: Rename 'wait_lock' to 'irq_lock'
The 'wait_lock' name seems to be a copy-paste from omapdrm, and makes no
sense here. Rename it to 'irq_lock'. Also clarify the related comment to
make it clear what it protects, and drop any comments related to
'wait_list' which doesn't exist in tidss.
Reviewed-by: Devarsh Thakkar <devarsht@ti.com>
Reviewed-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241021-tidss-irq-fix-v1-7-82ddaec94e4a@ideasonboard.com
show more ...
|
| a9a73f26 | 21-Oct-2024 |
Devarsh Thakkar <devarsht@ti.com> |
drm/tidss: Fix race condition while handling interrupt registers
The driver has a spinlock for protecting the irq_masks field and irq
enable registers. However, the driver misses protecting the irq
drm/tidss: Fix race condition while handling interrupt registers
The driver has a spinlock for protecting the irq_masks field and irq
enable registers. However, the driver misses protecting the irq status
registers which can lead to races.
Take the spinlock when accessing irqstatus too.
Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")
Cc: stable@vger.kernel.org
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
[Tomi: updated the desc]
Reviewed-by: Jonathan Cormier <jcormier@criticallink.com>
Tested-by: Jonathan Cormier <jcormier@criticallink.com>
Reviewed-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241021-tidss-irq-fix-v1-6-82ddaec94e4a@ideasonboard.com
show more ...
|
| 361a2ebb | 21-Oct-2024 |
Devarsh Thakkar <devarsht@ti.com> |
drm/tidss: Clear the interrupt status for interrupts being disabled
The driver does not touch the irqstatus register when it is disabling
interrupts. This might cause an interrupt to trigger for an
drm/tidss: Clear the interrupt status for interrupts being disabled
The driver does not touch the irqstatus register when it is disabling
interrupts. This might cause an interrupt to trigger for an interrupt
that was just disabled.
To fix the issue, clear the irqstatus registers right after disabling
the interrupts.
Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")
Cc: stable@vger.kernel.org
Reported-by: Jonathan Cormier <jcormier@criticallink.com>
Closes: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1394222/am625-issue-about-tidss-rcu_preempt-self-detected-stall-on-cpu/5424479#5424479
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
[Tomi: mostly rewrote the patch]
Reviewed-by: Jonathan Cormier <jcormier@criticallink.com>
Tested-by: Jonathan Cormier <jcormier@criticallink.com>
Reviewed-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241021-tidss-irq-fix-v1-5-82ddaec94e4a@ideasonboard.com
show more ...
|
| 76bae5b9 | 21-Oct-2024 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Add printing of underflows
Add printing of underflows the same way as we handle sync losts.
Reviewed-by: Devarsh Thakkar <devarsht@ti.com>
Reviewed-by: Aradhya Bhatia <aradhya.bhatia@lin
drm/tidss: Add printing of underflows
Add printing of underflows the same way as we handle sync losts.
Reviewed-by: Devarsh Thakkar <devarsht@ti.com>
Reviewed-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241021-tidss-irq-fix-v1-4-82ddaec94e4a@ideasonboard.com
show more ...
|
| f8e59e62 | 21-Oct-2024 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Remove extra K2G check
We check if the platform is K2G in dispc_k3_clear_irqstatus(), and
return early if so. This cannot happen, as the _k3_ functions are never
called on K2G in the firs
drm/tidss: Remove extra K2G check
We check if the platform is K2G in dispc_k3_clear_irqstatus(), and
return early if so. This cannot happen, as the _k3_ functions are never
called on K2G in the first place. So remove the check.
Reviewed-by: Devarsh Thakkar <devarsht@ti.com>
Reviewed-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241021-tidss-irq-fix-v1-3-82ddaec94e4a@ideasonboard.com
show more ...
|
| 18f430ac | 21-Oct-2024 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Remove unused OCP error flag
We never use the DSS_IRQ_DEVICE_OCP_ERR flag, and the HW doesn't even
have such a bit... So remove it.
Reviewed-by: Devarsh Thakkar <devarsht@ti.com>
Reviewe
drm/tidss: Remove unused OCP error flag
We never use the DSS_IRQ_DEVICE_OCP_ERR flag, and the HW doesn't even
have such a bit... So remove it.
Reviewed-by: Devarsh Thakkar <devarsht@ti.com>
Reviewed-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241021-tidss-irq-fix-v1-2-82ddaec94e4a@ideasonboard.com
show more ...
|
| 44b6730a | 21-Oct-2024 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Fix issue in irq handling causing irq-flood issue
It has been observed that sometimes DSS will trigger an interrupt and
the top level interrupt (DISPC_IRQSTATUS) is not zero, but the VP a
drm/tidss: Fix issue in irq handling causing irq-flood issue
It has been observed that sometimes DSS will trigger an interrupt and
the top level interrupt (DISPC_IRQSTATUS) is not zero, but the VP and
VID level interrupt-statuses are zero.
As the top level irqstatus is supposed to tell whether we have VP/VID
interrupts, the thinking of the driver authors was that this particular
case could never happen. Thus the driver only clears the DISPC_IRQSTATUS
bits which has corresponding interrupts in VP/VID status. So when this
issue happens, the driver will not clear DISPC_IRQSTATUS, and we get an
interrupt flood.
It is unclear why the issue happens. It could be a race issue in the
driver, but no such race has been found. It could also be an issue with
the HW. However a similar case can be easily triggered by manually
writing to DISPC_IRQSTATUS_RAW. This will forcibly set a bit in the
DISPC_IRQSTATUS and trigger an interrupt, and as the driver never clears
the bit, we get an interrupt flood.
To fix the issue, always clear DISPC_IRQSTATUS. The concern with this
solution is that if the top level irqstatus is the one that triggers the
interrupt, always clearing DISPC_IRQSTATUS might leave some interrupts
unhandled if VP/VID interrupt statuses have bits set. However, testing
shows that if any of the irqstatuses is set (i.e. even if
DISPC_IRQSTATUS == 0, but a VID irqstatus has a bit set), we will get an
interrupt.
Co-developed-by: Bin Liu <b-liu@ti.com>
Signed-off-by: Bin Liu <b-liu@ti.com>
Co-developed-by: Devarsh Thakkar <devarsht@ti.com>
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
Co-developed-by: Jonathan Cormier <jcormier@criticallink.com>
Signed-off-by: Jonathan Cormier <jcormier@criticallink.com>
Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")
Cc: stable@vger.kernel.org
Tested-by: Jonathan Cormier <jcormier@criticallink.com>
Reviewed-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241021-tidss-irq-fix-v1-1-82ddaec94e4a@ideasonboard.com
show more ...
|
| c079e2e1 | 13-Feb-2024 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Fix sync-lost issue with two displays
A sync lost issue can be observed with two displays, when moving a plane
from one disabled display to an another disabled display, and then
enabling
drm/tidss: Fix sync-lost issue with two displays
A sync lost issue can be observed with two displays, when moving a plane
from one disabled display to an another disabled display, and then
enabling the display to which the plane was moved to. The exact
requirements for this to trigger are not clear.
It looks like the issue is that the layers are left enabled in the first
display's OVR registers. Even if the corresponding VP is disabled, it
still causes an issue, as if the disabled VP and its OVR would still be
in use, leading to the same VID being used by two OVRs. However, this is
just speculation based on testing the DSS behavior.
Experimentation shows that as a workaround, we can disable all the
layers in the OVR when disabling a VP. There should be no downside to
this, as the OVR is anyway effectively disabled if its VP is disabled,
and it seems to solve the sync lost issue.
However, there may be a bigger issue in play here, related to J721e
erratum i2097 ("DSS: Disabling a Layer Connected to Overlay May Result
in Synclost During the Next Frame"). Experimentation also shows that the
OVR's CHANNELIN field has similar issue. So we may need to revisit this
when we find out more about the core issue.
Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240213-tidss-fixes-v1-2-d709e8dfa505@ideasonboard.com
show more ...
|
| ca89b697 | 09-Nov-2023 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Use DRM_PLANE_COMMIT_ACTIVE_ONLY
At the moment the driver does not use DRM_PLANE_COMMIT_ACTIVE_ONLY, but
still checks for crtc->state->active in tidss_crtc_atomic_flush(), and
skips the f
drm/tidss: Use DRM_PLANE_COMMIT_ACTIVE_ONLY
At the moment the driver does not use DRM_PLANE_COMMIT_ACTIVE_ONLY, but
still checks for crtc->state->active in tidss_crtc_atomic_flush(), and
skips the flush if the crtc is not active.
The exact reason why DRM_PLANE_COMMIT_ACTIVE_ONLY is not used has been
lost in history. DRM_PLANE_COMMIT_ACTIVE_ONLY does also affect the plane
updates, and I think the issue was related to multi-display systems and
moving planes between the displays. However, it is possible the issue
was only present on the older DSS hardware, handled by the omapdrm
driver (on which the tidss driver is loosely based).
Reviewing the code related to DRM_PLANE_COMMIT_ACTIVE_ONLY does not show
any issues, and testing on J7 EVM with two displays works fine.
Change the driver to use DRM_PLANE_COMMIT_ACTIVE_ONLY.
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-11-ac91b5ea35c0@ideasonboard.com
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
show more ...
|
| 95d4b471 | 09-Nov-2023 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Fix atomic_flush check
tidss_crtc_atomic_flush() checks if the crtc is enabled, and if not,
returns immediately as there's no reason to do any register changes.
However, the code checks
drm/tidss: Fix atomic_flush check
tidss_crtc_atomic_flush() checks if the crtc is enabled, and if not,
returns immediately as there's no reason to do any register changes.
However, the code checks for 'crtc->state->enable', which does not
reflect the actual HW state. We should instead look at the
'crtc->state->active' flag.
This causes the tidss_crtc_atomic_flush() to proceed with the flush even
if the active state is false, which then causes us to hit the
WARN_ON(!crtc->state->event) check.
Fix this by checking the active flag, and while at it, fix the related
debug print which had "active" and "needs modeset" wrong way.
Cc: <stable@vger.kernel.org>
Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-10-ac91b5ea35c0@ideasonboard.com
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
show more ...
|
| d4652187 | 09-Nov-2023 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: IRQ code cleanup
The IRQ setup code is overly complex. All we really need to do is
initialize the related fields in struct tidss_device, and request the
IRQ.
We can drop all the HW acces
drm/tidss: IRQ code cleanup
The IRQ setup code is overly complex. All we really need to do is
initialize the related fields in struct tidss_device, and request the
IRQ.
We can drop all the HW accesses, as they are pointless: the driver will
set the IRQs correctly when it needs any of the IRQs, and at probe time
we have done a reset, so we know that all the IRQs are masked by default
in the hardware.
Thus we can combine the tidss_irq_preinstall() and
tidss_irq_postinstall() into the tidss_irq_install() function, drop the
HW accesses, and drop the use of spinlock, as this is done at init time
and there can be no races.
We can also drop the HW access from the tidss_irq_uninstall(), as the
driver will anyway disable and suspend the hardware at remove time.
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-9-ac91b5ea35c0@ideasonboard.com
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
show more ...
|
| bc288a92 | 09-Nov-2023 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Fix dss reset
The probe function calls dispc_softreset() before runtime PM is enabled
and without enabling any of the DSS clocks. This happens to work by
luck, and we need to make sure th
drm/tidss: Fix dss reset
The probe function calls dispc_softreset() before runtime PM is enabled
and without enabling any of the DSS clocks. This happens to work by
luck, and we need to make sure the DSS HW is active and the fclk is
enabled.
To fix the above, add a new function, dispc_init_hw(), which does:
- pm_runtime_set_active()
- clk_prepare_enable(fclk)
- dispc_softreset().
This ensures that the reset can be successfully accomplished.
Note that we use pm_runtime_set_active(), not the normal
pm_runtime_get(). The reason for this is that at this point we haven't
enabled the runtime PM yet and also we don't want the normal resume
callback to be called: the dispc resume callback does some initial HW
setup, and it expects that the HW was off (no video ports are
streaming). If the bootloader has enabled the DSS and has set up a
boot time splash-screen, the DSS would be enabled and streaming which
might lead to issues with the normal resume callback.
Fixes: c9b2d923befd ("drm/tidss: Soft Reset DISPC on startup")
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-8-ac91b5ea35c0@ideasonboard.com
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
show more ...
|
| 576d96c5 | 09-Nov-2023 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Add simple K2G manual reset
K2G display controller does not support soft reset, but we can do the
most important steps manually: mask the IRQs and disable the VPs.
Reviewed-by: Aradhya B
drm/tidss: Add simple K2G manual reset
K2G display controller does not support soft reset, but we can do the
most important steps manually: mask the IRQs and disable the VPs.
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-7-ac91b5ea35c0@ideasonboard.com
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
show more ...
|
| 15182515 | 09-Nov-2023 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Check for K2G in in dispc_softreset()
K2G doesn't have softreset feature. Instead of having every caller of
dispc_softreset() check for K2G, move the check into dispc_softreset(),
and mak
drm/tidss: Check for K2G in in dispc_softreset()
K2G doesn't have softreset feature. Instead of having every caller of
dispc_softreset() check for K2G, move the check into dispc_softreset(),
and make dispc_softreset() return 0 in case of K2G.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-6-ac91b5ea35c0@ideasonboard.com
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
show more ...
|
| aceafbb5 | 09-Nov-2023 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Return error value from from softreset
Return an error value from dispc_softreset() so that the caller can
handle the errors.
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Link: https:/
drm/tidss: Return error value from from softreset
Return an error value from dispc_softreset() so that the caller can
handle the errors.
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-5-ac91b5ea35c0@ideasonboard.com
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
show more ...
|
| 36d1e085 | 09-Nov-2023 |
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> |
drm/tidss: Move reset to the end of dispc_init()
We do a DSS reset in the middle of the dispc_init(). While that happens
to work now, we should really make sure that e..g the fclk, which is
acquired
drm/tidss: Move reset to the end of dispc_init()
We do a DSS reset in the middle of the dispc_init(). While that happens
to work now, we should really make sure that e..g the fclk, which is
acquired only later in the function, is enabled when doing a reset. This
will be handled in a later patch, but for now, let's move the
dispc_softreset() call to the end of dispc_init(), which is a sensible
place for it anyway.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com>
Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-4-ac91b5ea35c0@ideasonboard.com
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
show more ...
|