sparse reports:
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c:122:16: warning: incorrect type in argument 1 (different address spaces)
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c:122:16: expected void const volatile [noderef] __iomem *addr
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c:122:16: got unsigned int [usertype] *wa_dma_data
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c:130:9: warning: incorrect type in argument 2 (different address spaces)
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c:130:9: expected void volatile [noderef] __iomem *addr
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c:130:9: got unsigned int [usertype] *wa_dma_data
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c:414:9: warning: incorrect type in argument 1 (different address spaces)
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c:414:9: expected void const volatile [noderef] __iomem *addr
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c:414:9: got unsigned int *
These come from pieces of code which do essentially:
p = dma_alloc_coherent()
dma_transfer_to_p()
readl(p)
writel(x, p)
dma_transfer_from_p()
I think we would do just fine without readl() and writel(), accessing
the memory without any extras, but ensuring that the necessary barriers
are in place. But this code is for a legacy platform, has been working
for ages, and it's doing work-arounds for hardware issues, and those
hardware issues are very difficult to trigger... So I would just rather
leave the code be as it is now.
However, the warnings are not nice. Hide the warnings by a (__iomem void
*) typecast.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202407311737.VsJ0Sr1w-lkp@intel.com/
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240806-omapdrm-misc-fixes-v1-2-15d31aea0831@ideasonboard.com
smatch reports:
drivers/gpu/drm/omapdrm/dss/base.c:176 omapdss_device_disconnect() error: we previously assumed 'src' could be null (see line 169)
This code is mostly from a time when omapdrm had its own display device
model. I can't honestly remember the details, and I don't think it's
worth digging in deeply into that for a legacy driver.
However, it looks like we only call omapdss_device_disconnect() and
omapdss_device_connect() with NULL as the src parameter. We can thus
drop the src parameter from both functions, and fix the smatch warning.
I don't think omapdss_device_disconnect() ever gets NULL for the dst
parameter (if it did, we'd crash soon after returning from the
function), but I have kept the !dst check, just in case, but I added a
WARN_ON() there.
Also, if the dst parameter can be NULL, we can't always get the struct
dss_device pointer from dst->dss (which is only used for a debug print).
To make sure we can't hit that issue, do it similarly to the
omapdss_device_connect() function: add 'struct dss_device *dss' as the
first parameter, so that we always have it regardless of the dst.
Fixes: 79107f274b ("drm/omap: Add support for drm_bridge")
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240806-omapdrm-misc-fixes-v1-1-15d31aea0831@ideasonboard.com
Inline ast_vga_connector_init() into its only caller. The helper
currently only does half of the connector-init work and is trivial
enough to be inlined. While at it, remove the error message from the
call to ast_ddc_create(). The function already warns on errors.
Also set the local variables for encoder and connector as late as
possible, so that the compiler warns if we use them before having
initialized the instance.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240911115347.899148-9-tzimmermann@suse.de
Inline ast_sil164_connector_init() into its only caller. The helper
currently only does half of the connector-init work and is trivial
enough to be inlined. While at it, remove the error message from the
call to ast_ddc_create(). The function already warns on errors.
Also set the local variables for encoder and connector as late as
possible, so that the compiler warns if we use them before having
initialized the instance.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240911115347.899148-8-tzimmermann@suse.de
Inline ast_dp501_connector_init() into its only caller. The helper
currently only does half of the connector-init work and is trivial
enough to be inlined.
Also set the local variables for encoder and connector as late as
possible, so that the compiler warns if we use them before having
initialized the instance.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240911115347.899148-6-tzimmermann@suse.de
Replace ast_dp_set_on_off() with ast_dp_set_enable(). The helper's
new name reflects the performed operation. If enabling fails, the
new helper prints a warning. The code that waits for the programmed
effect to take place is now located in __ast_dp_wait_enable().
Also align the register constants with the rest of the code.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240911115347.899148-5-tzimmermann@suse.de
Replace the helper for controlling power on the physical connector,
ast_dp_power_on_off(), with ast_dp_set_phy_sleep(). The new name
reflects the effect of the operation. Simplify the implementation.
The call now controls sleeping, hence semantics are inversed. Each
'on' becomes an 'off' operation and vice versa.
Do the same for ast_dp_power_is_on() and also align naming of the
register constant with the rest of the code.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240911115347.899148-4-tzimmermann@suse.de
Inline ast_astdp_connector_init() into its only caller. The helper
currently only does half of the connector-init work and is trivial
enough to be inlined.
Also set the local variables for encoder and connector as late as
possible, so that the compiler warns if we use them before having
initialized the instance.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240911115347.899148-2-tzimmermann@suse.de
Opt for devm_ioremap_wc() over devm_ioremap() when mapping the framebuffer.
Using devm_ioremap() results in the VA being mapped with PAT=UC-, which
considerably slows down drm_fb_memcpy(). In contrast, devm_ioremap_wc()
maps the VA with PAT set to WC, leading to better performance on platforms
where access to UC memory is much slower than WC memory.
Here's the performance data measured in a guest on the physical machine
"Sapphire Rapids XCC".
With host KVM honors guest PAT memory types, the effective memory type
for this framebuffer range is
- WC when devm_ioremap_wc() is used
- UC- when devm_ioremap() is used.
The data presented is an average from 10 execution runs.
Cycles: Avg cycles of executed bochs_primary_plane_helper_atomic_update()
from VM boot to GDM show up
Cnt: Avg cnt of executed bochs_primary_plane_helper_atomic_update()
from VM boot to GDM show up
T: Avg time of each bochs_primary_plane_helper_atomic_update().
-------------------------------------------------
| | devm_ioremap() | devm_ioremap_wc() |
|------------|----------------|-------------------|
| Cycles | 211.545M | 0.157M |
|------------|----------------|-------------------|
| Cnt | 142 | 1917 |
|------------|----------------|-------------------|
| T | 0.1748s | 0.0004s |
-------------------------------------------------
Note:
Following the rebase to [3], the previously reported GDM failure on the
VGA device [1] can no longer be reproduced, thanks to the memory management
improvements made in [2]. Despite this, I have proceeded to submit this
patch because of the noticeable performance improvements it provides.
Reported-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Closes: https://lore.kernel.org/all/87jzfutmfc.fsf@redhat.com/#t
Cc: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
Link: https://lore.kernel.org/all/87jzfutmfc.fsf@redhat.com/#t [1]
Link: https://patchwork.freedesktop.org/series/138086 [2]
Link: https://gitlab.freedesktop.org/drm/misc/kernel/-/tree/drm-misc-next [3]
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Tested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://patchwork.freedesktop.org/patch/msgid/20240909131643.28915-1-yan.y.zhao@intel.com
Add three KUnit test cases for the drm_framebuffer_init function:
1. Test if expected values are being set after drm_framebuffer_init() call.
2. Try to init a framebuffer without setting its format.
3. Try calling drm_framebuffer_init() with mismatch of the drm_device
passed at the first argument and the one pointed by fb->dev.
Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240911001559.28284-9-gcarlos@disroot.org
Signed-off-by: Maxime Ripard <mripard@kernel.org>
The dev_private member of drm_device is deprecated and its use should
be avoided. Stop using it by embedding the drm_device onto a mock struct.
The new mock struct allows to share variables and even further mocks
over the tests in a cleaner way than using dev_private void pointer.
Also start using drm_kunit_helper_alloc_drm_device() for allocating
the drm_device mock.
Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
Acked-by: Maxime Ripard <mripard@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20240911001559.28284-2-gcarlos@disroot.org
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Thomas needs 5a498d4d06 ("drm/fbdev-dma: Only install deferred I/O
if necessary") in drm-misc, so start the backmerge cascade.
Signed-off-by: Simona Vetter <simona.vetter@ffwll.ch>
