kernel/linux.git/drivers/gpu/drm/tidss, branch linux-6.9.y

drm/tidss: Fix sync-lost issue with two displays

2024-02-26T08:09:43+00:00

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 Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/20240213-tidss-fixes-v1-2-d709e8dfa505@ideasonboard.com

drm/tidss: Fix initial plane zpos values

2024-02-26T08:09:23+00:00

When the driver sets up the zpos property it sets the default zpos value to the HW id of the plane. That is fine as such, but as on many DSS versions the driver arranges the DRM planes in a different order than the HW planes (to keep the non-scalable planes first), this leads to odd initial zpos values. An example is J721e, where the initial zpos values for DRM planes are 1, 3, 0, 2. In theory the userspace should configure the zpos values properly when using multiple planes, and in that sense the initial zpos values shouldn't matter, but there's really no reason not to fix this and help the userspace apps which don't handle zpos perfectly. In particular, some versions of Weston seem to have issues dealing with the planes with the current default zpos values. So let's change the zpos values for the DRM planes to 0, 1, 2, 3. Another option would be to configure the planes marked as primary planes to zpos 0. On a two display system this would give us plane zpos values of 0, 0, 1, 2. The end result and behavior would be very similar in this option, and I'm not aware that this would actually help us in any way. So, to keep the code simple, I opted for the 0, 1, 2, 3 values. Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem") Reviewed-by: Aradhya Bhatia Signed-off-by: Tomi Valkeinen Link: https://patchwork.freedesktop.org/patch/msgid/20240213-tidss-fixes-v1-1-d709e8dfa505@ideasonboard.com

drm/tidss: Use DRM_PLANE_COMMIT_ACTIVE_ONLY

2023-12-01T10:04:43+00:00

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 Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-11-ac91b5ea35c0@ideasonboard.com Signed-off-by: Tomi Valkeinen

drm/tidss: Fix atomic_flush check

2023-12-01T10:04:43+00:00

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: Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem") Reviewed-by: Aradhya Bhatia Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-10-ac91b5ea35c0@ideasonboard.com Signed-off-by: Tomi Valkeinen

drm/tidss: IRQ code cleanup

2023-12-01T10:04:42+00:00

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 Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-9-ac91b5ea35c0@ideasonboard.com Signed-off-by: Tomi Valkeinen

drm/tidss: Fix dss reset

2023-12-01T10:04:42+00:00

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 Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-8-ac91b5ea35c0@ideasonboard.com Signed-off-by: Tomi Valkeinen

drm/tidss: Add simple K2G manual reset

2023-12-01T10:04:42+00:00

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 Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-7-ac91b5ea35c0@ideasonboard.com Signed-off-by: Tomi Valkeinen

drm/tidss: Check for K2G in in dispc_softreset()

2023-12-01T10:04:42+00:00

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 Reviewed-by: Aradhya Bhatia Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-6-ac91b5ea35c0@ideasonboard.com Signed-off-by: Tomi Valkeinen

drm/tidss: Return error value from from softreset

2023-12-01T10:04:42+00:00

Return an error value from dispc_softreset() so that the caller can handle the errors. Reviewed-by: Aradhya Bhatia Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-5-ac91b5ea35c0@ideasonboard.com Signed-off-by: Tomi Valkeinen

drm/tidss: Move reset to the end of dispc_init()

2023-12-01T10:04:42+00:00

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 Reviewed-by: Aradhya Bhatia Link: https://lore.kernel.org/r/20231109-tidss-probe-v2-4-ac91b5ea35c0@ideasonboard.com Signed-off-by: Tomi Valkeinen