Re: [PATCH v3] drm/atomic-helpers: Invoke end_fb_access while owning plane state
Hi
Am 03.12.23 um 21:57 schrieb Alyssa Ross:
Thomas Zimmermann writes:
Invoke drm_plane_helper_funcs.end_fb_access before
drm_atomic_helper_commit_hw_done(). The latter function hands over
ownership of the plane state to the following commit, which might
free it. Releasing resources in end_fb_access then operates on undefined
state. This bug has been observed with non-blocking commits when they
are being queued up quickly.
Here is an example stack trace from the bug report. The plane state has
been free'd already, so the pages for drm_gem_fb_vunmap() are gone.
Unable to handle kernel paging request at virtual address 00010049
[...]
drm_gem_fb_vunmap+0x18/0x74
drm_gem_end_shadow_fb_access+0x1c/0x2c
drm_atomic_helper_cleanup_planes+0x58/0xd8
drm_atomic_helper_commit_tail+0x90/0xa0
commit_tail+0x15c/0x188
commit_work+0x14/0x20
Fix this by running end_fb_access immediately after updating all planes
in drm_atomic_helper_commit_planes(). The existing clean-up helper
drm_atomic_helper_cleanup_planes() now only handles cleanup_fb.
For aborted commits, roll back from drm_atomic_helper_prepare_planes()
in the new helper drm_atomic_helper_unprepare_planes(). This case is
different from regular cleanup, as we have to release the new state;
regular cleanup releases the old state. The new helper also invokes
cleanup_fb for all planes.
The changes mostly involve DRM's atomic helpers. Only two drivers, i915
and nouveau, implement their own commit function. Update them to invoke
drm_atomic_helper_unprepare_planes(). Drivers with custom commit_tail
function do not require changes.
v3:
* add drm_atomic_helper_unprepare_planes() for rolling back
* use correct state for end_fb_access
v2:
* fix test in drm_atomic_helper_cleanup_planes()
Reported-by: Alyssa Ross
Closes: https://lore.kernel.org/dri-devel/87leazm0ya@alyssa.is/
Suggested-by: Daniel Vetter
Fixes: 94d879eaf7fb ("drm/atomic-helper: Add {begin,end}_fb_access to plane
helpers")
Signed-off-by: Thomas Zimmermann
Cc: # v6.2+
I've been running this for days now, and haven't had a single Oops.
Given the rate with which I encountered them before in this
configuration, it looks very likely that the issue is resolved.
Tested-by: Alyssa Ross
And, once the wrong parameter name in the kerneldoc identified by the
kernel test robot is resolved,
Reviewed-by: Alyssa Ross
Great. I'll prepare another update so this fix can land before the next
-fixes PR. Thanks a lot!
Best regards
Thomas
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
OpenPGP_signature.asc
Description: OpenPGP digital signature
[PATCH] drm/vmwgfx: fix a memleak in vmw_gmrid_man_get_node
When ida_alloc_max fails, resources allocated before should be freed,
including *res allocated by kmalloc and ttm_resource_init.
Fixes: d3bcb4b02fe9 ("drm/vmwgfx: switch the TTM backends to self alloc")
Signed-off-by: Zhipeng Lu
---
drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
index ceb4d3d3b965..220561b73493 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
@@ -65,6 +65,8 @@ static int vmw_gmrid_man_get_node(struct ttm_resource_manager
*man,
id = ida_alloc_max(&gman->gmr_ida, gman->max_gmr_ids - 1, GFP_KERNEL);
if (id < 0)
+ ttm_resource_fini(man, *res);
+ kfree(*res);
return id;
spin_lock(&gman->lock);
--
2.34.1
[PATCH] drm/radeon/trinity_dpm: fix a memleak in trinity_parse_power_table
The rdev->pm.dpm.ps allocated by kcalloc should be freed in every
following error-handling path. However, in the error-handling of
rdev->pm.power_state[i].clock_info the rdev->pm.dpm.ps is not freed,
resulting in a memleak in this function.
Fixes: d70229f70447 ("drm/radeon/kms: add dpm support for trinity asics")
Signed-off-by: Zhipeng Lu
---
drivers/gpu/drm/radeon/trinity_dpm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/radeon/trinity_dpm.c
b/drivers/gpu/drm/radeon/trinity_dpm.c
index 08ea1c864cb2..8bf56fb7b933 100644
--- a/drivers/gpu/drm/radeon/trinity_dpm.c
+++ b/drivers/gpu/drm/radeon/trinity_dpm.c
@@ -1727,6 +1727,7 @@ static int trinity_parse_power_table(struct radeon_device
*rdev)
non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *)
&non_clock_info_array->nonClockInfo[non_clock_array_index];
if (!rdev->pm.power_state[i].clock_info)
+ kfree(rdev->pm.dpm.ps);
return -EINVAL;
ps = kzalloc(sizeof(struct sumo_ps), GFP_KERNEL);
if (ps == NULL) {
--
2.34.1
[PATCH] drm/radeon/dpm: fix a memleak in sumo_parse_power_table
The rdev->pm.dpm.ps allocated by kcalloc should be freed in every
following error-handling path. However, in the error-handling of
rdev->pm.power_state[i].clock_info the rdev->pm.dpm.ps is not freed,
resulting in a memleak in this function.
Fixes: 80ea2c129c76 ("drm/radeon/kms: add dpm support for sumo asics (v2)")
Signed-off-by: Zhipeng Lu
---
drivers/gpu/drm/radeon/sumo_dpm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/radeon/sumo_dpm.c
b/drivers/gpu/drm/radeon/sumo_dpm.c
index f74f381af05f..bde640053708 100644
--- a/drivers/gpu/drm/radeon/sumo_dpm.c
+++ b/drivers/gpu/drm/radeon/sumo_dpm.c
@@ -1494,6 +1494,7 @@ static int sumo_parse_power_table(struct radeon_device
*rdev)
non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *)
&non_clock_info_array->nonClockInfo[non_clock_array_index];
if (!rdev->pm.power_state[i].clock_info)
+ kfree(rdev->pm.dpm.ps);
return -EINVAL;
ps = kzalloc(sizeof(struct sumo_ps), GFP_KERNEL);
if (ps == NULL) {
--
2.34.1
Re: [PATCH] drm/radeon/dpm: fix a memleak in sumo_parse_power_table
Am 03.12.23 um 18:16 schrieb Zhipeng Lu:
The rdev->pm.dpm.ps allocated by kcalloc should be freed in every
following error-handling path. However, in the error-handling of
rdev->pm.power_state[i].clock_info the rdev->pm.dpm.ps is not freed,
resulting in a memleak in this function.
Fixes: 80ea2c129c76 ("drm/radeon/kms: add dpm support for sumo asics (v2)")
Signed-off-by: Zhipeng Lu
---
drivers/gpu/drm/radeon/sumo_dpm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/radeon/sumo_dpm.c
b/drivers/gpu/drm/radeon/sumo_dpm.c
index f74f381af05f..bde640053708 100644
--- a/drivers/gpu/drm/radeon/sumo_dpm.c
+++ b/drivers/gpu/drm/radeon/sumo_dpm.c
@@ -1494,6 +1494,7 @@ static int sumo_parse_power_table(struct radeon_device
*rdev)
non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *)
&non_clock_info_array->nonClockInfo[non_clock_array_index];
if (!rdev->pm.power_state[i].clock_info)
+ kfree(rdev->pm.dpm.ps);
return -EINVAL;
That change is obviously not correct since you now always return -EINVAL.
You need to at least add {} here.
Regards,
Christian.
ps = kzalloc(sizeof(struct sumo_ps), GFP_KERNEL);
if (ps == NULL) {
[PATCH v4] drm/atomic-helpers: Invoke end_fb_access while owning plane state
Invoke drm_plane_helper_funcs.end_fb_access before
drm_atomic_helper_commit_hw_done(). The latter function hands over
ownership of the plane state to the following commit, which might
free it. Releasing resources in end_fb_access then operates on undefined
state. This bug has been observed with non-blocking commits when they
are being queued up quickly.
Here is an example stack trace from the bug report. The plane state has
been free'd already, so the pages for drm_gem_fb_vunmap() are gone.
Unable to handle kernel paging request at virtual address 00010049
[...]
drm_gem_fb_vunmap+0x18/0x74
drm_gem_end_shadow_fb_access+0x1c/0x2c
drm_atomic_helper_cleanup_planes+0x58/0xd8
drm_atomic_helper_commit_tail+0x90/0xa0
commit_tail+0x15c/0x188
commit_work+0x14/0x20
Fix this by running end_fb_access immediately after updating all planes
in drm_atomic_helper_commit_planes(). The existing clean-up helper
drm_atomic_helper_cleanup_planes() now only handles cleanup_fb.
For aborted commits, roll back from drm_atomic_helper_prepare_planes()
in the new helper drm_atomic_helper_unprepare_planes(). This case is
different from regular cleanup, as we have to release the new state;
regular cleanup releases the old state. The new helper also invokes
cleanup_fb for all planes.
The changes mostly involve DRM's atomic helpers. Only two drivers, i915
and nouveau, implement their own commit function. Update them to invoke
drm_atomic_helper_unprepare_planes(). Drivers with custom commit_tail
function do not require changes.
v4:
* fix documentation (kernel test robot)
v3:
* add drm_atomic_helper_unprepare_planes() for rolling back
* use correct state for end_fb_access
v2:
* fix test in drm_atomic_helper_cleanup_planes()
Reported-by: Alyssa Ross
Closes: https://lore.kernel.org/dri-devel/87leazm0ya@alyssa.is/
Suggested-by: Daniel Vetter
Fixes: 94d879eaf7fb ("drm/atomic-helper: Add {begin,end}_fb_access to plane
helpers")
Tested-by: Alyssa Ross
Reviewed-by: Alyssa Ross
Signed-off-by: Thomas Zimmermann
Cc: # v6.2+
---
drivers/gpu/drm/drm_atomic_helper.c | 78 +---
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
include/drm/drm_atomic_helper.h | 2 +
4 files changed, 56 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index c3f677130def0..a920fbae714c8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2012,7 +2012,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
return ret;
drm_atomic_helper_async_commit(dev, state);
- drm_atomic_helper_cleanup_planes(dev, state);
+ drm_atomic_helper_unprepare_planes(dev, state);
return 0;
}
@@ -2072,7 +2072,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
return 0;
err:
- drm_atomic_helper_cleanup_planes(dev, state);
+ drm_atomic_helper_unprepare_planes(dev, state);
return ret;
}
EXPORT_SYMBOL(drm_atomic_helper_commit);
@@ -2650,6 +2650,39 @@ int drm_atomic_helper_prepare_planes(struct drm_device
*dev,
}
EXPORT_SYMBOL(drm_atomic_helper_prepare_planes);
+/**
+ * drm_atomic_helper_unprepare_planes - release plane resources on aborts
+ * @dev: DRM device
+ * @state: atomic state object with old state structures
+ *
+ * This function cleans up plane state, specifically framebuffers, from the
+ * atomic state. It undoes the effects of drm_atomic_helper_prepare_planes()
+ * when aborting an atomic commit. For cleaning up after a successful commit
+ * use drm_atomic_helper_cleanup_planes().
+ */
+void drm_atomic_helper_unprepare_planes(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+ struct drm_plane *plane;
+ struct drm_plane_state *new_plane_state;
+ int i;
+
+ for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+ const struct drm_plane_helper_funcs *funcs =
plane->helper_private;
+
+ if (funcs->end_fb_access)
+ funcs->end_fb_access(plane, new_plane_state);
+ }
+
+ for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+ const struct drm_plane_helper_funcs *funcs =
plane->helper_private;
+
+ if (funcs->cleanup_fb)
+ funcs->cleanup_fb(plane, new_plane_state);
+ }
+}
+EXPORT_SYMBOL(drm_atomic_helper_unprepare_planes);
+
static bool plane_crtc_active(const struct drm_plane_state *state)
{
return state->crtc && state->crtc->state->active;
@@ -2784,6 +2817,17 @@ void drm_atomic_helper_commit_planes(struct drm_device
*dev,
funcs->atomic_flush(crtc, old_state);
}
+
+ /*
+* Signal end of framebuffer access here before hw_done. After h
Re: [PATCH RESEND] drm/atomic-helper: rename drm_atomic_helper_check_wb_encoder_state
On Sat, Dec 02, 2023 at 12:07:49AM +0200, Dmitry Baryshkov wrote:
> The drm_atomic_helper_check_wb_encoder_state() function doesn't use
> encoder for anything other than getting the drm_device instance. The
> function's description talks about checking the writeback connector
> state, not the encoder state. Moreover, there is no such thing as an
> encoder state, encoders generally do not have a state on their own.
>
> Drop the first argument and rename the function to
> drm_atomic_helper_check_wb_connector_state().
>
> Signed-off-by: Dmitry Baryshkov
> ---
>
> Resending, no reaction for two months
>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 10 --
> drivers/gpu/drm/vkms/vkms_writeback.c | 2 +-
> include/drm/drm_atomic_helper.h | 3 +--
> 3 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 2444fc33dd7c..d69591381f00 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -795,8 +795,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
>
> /**
> - * drm_atomic_helper_check_wb_encoder_state() - Check writeback encoder state
> - * @encoder: encoder state to check
> + * drm_atomic_helper_check_wb_connector_state() - Check writeback connector
> state
> * @conn_state: connector state to check
> *
> * Checks if the writeback connector state is valid, and returns an error if
> it
> @@ -806,8 +805,7 @@ EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
> * Zero for success or -errno
> */
> int
> -drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
> - struct drm_connector_state *conn_state)
> +drm_atomic_helper_check_wb_connector_state(struct drm_connector_state
> *conn_state)
AFAIK, all the helpers take the object as first argument, so I'm fine
with the name change but it should take a drm_connector too. And ideally
a drm_atomic_state pointer instead of drm_connector_state too.
> {
> struct drm_writeback_job *wb_job = conn_state->writeback_job;
> struct drm_property_blob *pixel_format_blob;
> @@ -827,11 +825,11 @@ drm_atomic_helper_check_wb_encoder_state(struct
> drm_encoder *encoder,
> if (fb->format->format == formats[i])
> return 0;
>
> - drm_dbg_kms(encoder->dev, "Invalid pixel format %p4cc\n",
> &fb->format->format);
> + drm_dbg_kms(conn_state->connector->dev, "Invalid pixel format %p4cc\n",
> &fb->format->format);
Which would also avoid the checkpatch warning there.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH] drm/qxl: remove unused declaration
On Fri, 10 Nov 2023 13:50:31 +0800, heminhong wrote: > Some functions are never used by the driver, > removing the functions declaration, it can be reducing program size, > and improving code readability and maintainability. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: (subset) [PATCH RESEND] drm/drv: propagate errors from drm_modeset_register_all()
On Sun, 03 Dec 2023 01:55:52 +0300, Dmitry Baryshkov wrote: > In case the drm_modeset_register_all() function fails, its error code > will be ignored. Instead make the drm_dev_register() bail out in case of > such an error. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: Friendly ping. I think this patch was forgotten.//回复: [PATCH] drm/qxl: remove unused declaration
Applied, thanks for the reminder signature.asc Description: PGP signature
Re: [PATCH v5 00/32] drm/amd/display: add AMD driver-specific properties for color mgmt
On Fri, Dec 01, 2023 at 10:20:40AM -0500, Harry Wentland wrote: > > > On 2023-11-30 06:34, Daniel Vetter wrote: > > On Tue, 28 Nov 2023 at 23:11, Harry Wentland wrote: > >> > >> On 2023-11-16 14:57, Melissa Wen wrote: > >>> Hello, > >>> > >>> This series extends the current KMS color management API with AMD > >>> driver-specific properties to enhance the color management support on > >>> AMD Steam Deck. The key additions to the color pipeline include: > >>> > >> > >> snip > >> > >>> Melissa Wen (18): > >>> drm/drm_mode_object: increase max objects to accommodate new color > >>> props > >>> drm/drm_property: make replace_property_blob_from_id a DRM helper > >>> drm/drm_plane: track color mgmt changes per plane > >> > >> If all patches are merged through amd-staging-drm-next I worry that > >> conflicts creep in if any code around replace_property_blob_from_id > >> changes in DRM. > >> > >> My plan is to merge DRM patches through drm-misc-next, as well > >> as include them in the amd-staging-drm-next merge. They should then > >> fall out at the next amd-staging-drm-next pull and (hopefully) > >> ensure that there is no conflict. > >> > >> If no objections I'll go ahead with that later this week. > > > > Double-merging tends to be the worst because git doesn't realize the > > commits match, which actually makes the conflicts worse when they > > happen (because the 3-way merge diff gets absolute confused by all the > > changed context and misplaces everything to the max). So please don't, > > _only_ every cherry-pick when a patch in -next is also needed in > > -fixes, and we didn't put it into the right tree. But even that is a > > bit tricky and should only be done by maintainers (using dim > > cherry-pick if it's drm-misc) because the conflicts tend to be bad and > > need to be sorted out with backmerges sooner than later. > > > > For this case merge everything through one tree with the right acks, > > pull to drm-next asap and then backmerge into the other tree. Here > > probably amdgpu-next with drm-misc maintainer acks for the 3 core > > patches. Or if amdgpu-next pull won't come for a while, put them into > > drm-misc-next and just wait a week until it's in drm-next, then > > forward amdgpu-next. > > > > Maxime, Maarten, Thomas, could I get an ACK from you for the three > DRM core patches and an ACK for pulling them through the AMD tree? Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH RESEND 1/5] drm/atomic: add private obj state to state dump
On Sun, 3 Dec 2023 03:05:28 +0300, Dmitry Baryshkov wrote: > The drm_atomic_print_new_state() already prints private object state via > drm_atomic_private_obj_print_state(). Add private object state dumping > to __drm_state_dump(), so that it is also included into drm_state_dump() > output and into debugfs/dri/N/state file. > > > [ ... ] Acked-by: Maxime Ripard Thanks! Maxime
[PATCH v2] drm/panel-edp: Add SDC ATNA45AF01
Add support for the SDC ATNA45AF01 panel.
Signed-off-by: Abel Vesa
---
Changes in v2:
- moved the panel entry in the proper place, as suggested by Doug
- Link to v1:
https://lore.kernel.org/r/20231201-x1e80100-drm-panel-edp-v1-1-ef9def711...@linaro.org
---
drivers/gpu/drm/panel/panel-edp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 825fa2a0d8a5..78565c99b54d 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1983,6 +1983,8 @@ static const struct edp_panel_entry edp_panels[] = {
EDP_PANEL_ENTRY('K', 'D', 'C', 0x0809, &delay_200_500_e50,
"KD116N2930A15"),
EDP_PANEL_ENTRY('K', 'D', 'B', 0x1120, &delay_200_500_e80_d50,
"116N29-30NK-C007"),
+ EDP_PANEL_ENTRY('S', 'D', 'C', 0x416d, &delay_100_500_e200,
"ATNA45AF01"),
+
EDP_PANEL_ENTRY('S', 'H', 'P', 0x1511, &delay_200_500_e50,
"LQ140M1JW48"),
EDP_PANEL_ENTRY('S', 'H', 'P', 0x1523, &sharp_lq140m1jw46.delay,
"LQ140M1JW46"),
EDP_PANEL_ENTRY('S', 'H', 'P', 0x154c, &delay_200_500_p2e100,
"LQ116M1JW10"),
---
base-commit: 5eda217cee887e595ba2265435862d585d399769
change-id: 20231201-x1e80100-drm-panel-edp-e94dcd8250eb
Best regards,
--
Abel Vesa
Re: [PATCH RESEND v2 1/3] drm/encoder: register per-encoder debugfs dir
Hi,
On Sun, Dec 03, 2023 at 02:53:13PM +0300, Dmitry Baryshkov wrote:
> Each of connectors and CRTCs used by the DRM device provides debugfs
> directory, which is used by several standard debugfs files and can
> further be extended by the driver. Add such generic debugfs directories
> for encoder.
>
> Reviewed-by: Neil Armstrong
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/drm_debugfs.c | 25 +
> drivers/gpu/drm/drm_encoder.c | 4
> drivers/gpu/drm/drm_internal.h | 9 +
> include/drm/drm_encoder.h | 16 +++-
> 4 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index f291fb4b359f..00406b4f3235 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -589,4 +589,29 @@ void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
> crtc->debugfs_entry = NULL;
> }
>
> +void drm_debugfs_encoder_add(struct drm_encoder *encoder)
> +{
> + struct drm_minor *minor = encoder->dev->primary;
> + struct dentry *root;
> + char *name;
> +
> + name = kasprintf(GFP_KERNEL, "encoder-%d", encoder->index);
> + if (!name)
> + return;
> +
> + root = debugfs_create_dir(name, minor->debugfs_root);
> + kfree(name);
> +
> + encoder->debugfs_entry = root;
> +
> + if (encoder->funcs->debugfs_init)
> + encoder->funcs->debugfs_init(encoder, root);
> +}
> +
> +void drm_debugfs_encoder_remove(struct drm_encoder *encoder)
> +{
> + debugfs_remove_recursive(encoder->debugfs_entry);
> + encoder->debugfs_entry = NULL;
> +}
> +
> #endif /* CONFIG_DEBUG_FS */
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 1143bc7f3252..8f2bc6a28482 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -30,6 +30,7 @@
> #include
>
> #include "drm_crtc_internal.h"
> +#include "drm_internal.h"
>
> /**
> * DOC: overview
> @@ -74,6 +75,8 @@ int drm_encoder_register_all(struct drm_device *dev)
> int ret = 0;
>
> drm_for_each_encoder(encoder, dev) {
> + drm_debugfs_encoder_add(encoder);
> +
> if (encoder->funcs && encoder->funcs->late_register)
> ret = encoder->funcs->late_register(encoder);
> if (ret)
> @@ -90,6 +93,7 @@ void drm_encoder_unregister_all(struct drm_device *dev)
> drm_for_each_encoder(encoder, dev) {
> if (encoder->funcs && encoder->funcs->early_unregister)
> encoder->funcs->early_unregister(encoder);
> + drm_debugfs_encoder_remove(encoder);
> }
> }
>
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index b12c463bc460..7df17e4b0513 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -194,6 +194,8 @@ void drm_debugfs_connector_remove(struct drm_connector
> *connector);
> void drm_debugfs_crtc_add(struct drm_crtc *crtc);
> void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
> void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
> +void drm_debugfs_encoder_add(struct drm_encoder *encoder);
> +void drm_debugfs_encoder_remove(struct drm_encoder *encoder);
> #else
> static inline void drm_debugfs_dev_fini(struct drm_device *dev)
> {
> @@ -231,6 +233,13 @@ static inline void drm_debugfs_crtc_crc_add(struct
> drm_crtc *crtc)
> {
> }
>
> +static inline void drm_debugfs_encoder_add(struct drm_encoder *encoder)
> +{
> +}
<- You need to insert a new line here.
Once fixed,
Acked-by: Maxime Ripard
Maxime
signature.asc
Description: PGP signature
Re: (subset) [PATCH RFC v7 00/10] Support for Solid Fill Planes
On Sun, Dec 03, 2023 at 08:10:31PM +0200, Dmitry Baryshkov wrote: > On Sun, 3 Dec 2023 at 14:15, Simon Ser wrote: > > > > On Saturday, December 2nd, 2023 at 22:41, Dmitry Baryshkov > > wrote: > > > > > On Fri, 27 Oct 2023 15:32:50 -0700, Jessica Zhang wrote: > > > > > > > Some drivers support hardware that have optimizations for solid fill > > > > planes. This series aims to expose these capabilities to userspace as > > > > some compositors have a solid fill flag (ex. SOLID_COLOR in the Android > > > > hardware composer HAL) that can be set by apps like the Android Gears > > > > test app. > > > > > > > > In order to expose this capability to userspace, this series will: > > > > > > > > [...] > > > > > > > > > Applied to drm-misc-next, thanks! > > > > Where are the IGT and userspace for this uAPI addition? > > Indeed. I checked that there are uABI acks/reviews, but I didn't check > these requirements. Frankly speaking, I thought that they were handled > already, before giving the ack. How should we proceed? Should I land a > revert? Yes Maxime signature.asc Description: PGP signature
Re: [PATCH v5 3/5] mm/gup: Introduce pin_user_pages_fd() for pinning shmem/hugetlbfs file pages (v5)
On 04.12.23 09:16, Christoph Hellwig wrote: On Thu, Nov 30, 2023 at 06:41:11AM +, Kasireddy, Vivek wrote: I see your concern. The word "file" does make it look like this API works with all kinds of files although it is meant to specifically work with files that belong to shmemfs or hugetlbfs. Since it is intended to work with memfds in particular, I'll rename this helper alloc_memfd_page(). I think it also makes sense to do s/file/memfd in this whole patch. Does this sound ok? That sounds much better, yes. And please also rename the new api to memfd_pin_user_pages。 asserts that this is true). gup.c also seems like a very odd place for such a helper. I only created this helper to cleanly separate lookup and creation and to reduce the level of indentation in pin_user_pages_fd(). Anyway, would mm/memfd.c be a more appropriate location? I think so, but I'll defer to the MM maintainers. As mentioned above, this API is mainly intended for memfds and FWICS, memfds are backed by files from either shmemfs or hugetlbfs. Ok. Witht better naming this should be more obvious. All sounds reasonable to me! -- Cheers, David / dhildenb
[PATCH v2 1/8] drm/plane-helper: Move drm_plane_helper_atomic_check() into udl
The udl driver is the only caller of drm_plane_helper_atomic_check().
Move the function into the driver. No functional changes.
v2:
* fix documenation (Sui)
Signed-off-by: Thomas Zimmermann
Acked-by: Sui Jingfeng
---
drivers/gpu/drm/drm_crtc_helper.c | 7 ++-
drivers/gpu/drm/drm_plane_helper.c | 32 --
drivers/gpu/drm/udl/udl_modeset.c | 19 --
include/drm/drm_plane_helper.h | 2 --
4 files changed, 19 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index a209659a996c7..2dafc39a27cb9 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -439,11 +439,8 @@ EXPORT_SYMBOL(drm_crtc_helper_set_mode);
* @state: atomic state object
*
* Provides a default CRTC-state check handler for CRTCs that only have
- * one primary plane attached to it.
- *
- * This is often the case for the CRTC of simple framebuffers. See also
- * drm_plane_helper_atomic_check() for the respective plane-state check
- * helper function.
+ * one primary plane attached to it. This is often the case for the CRTC
+ * of simple framebuffers.
*
* RETURNS:
* Zero on success, or an errno code otherwise.
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 5e95089676ff8..7982be4b0306d 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -279,35 +279,3 @@ void drm_plane_helper_destroy(struct drm_plane *plane)
kfree(plane);
}
EXPORT_SYMBOL(drm_plane_helper_destroy);
-
-/**
- * drm_plane_helper_atomic_check() - Helper to check plane atomic-state
- * @plane: plane to check
- * @state: atomic state object
- *
- * Provides a default plane-state check handler for planes whose atomic-state
- * scale and positioning are not expected to change since the plane is always
- * a fullscreen scanout buffer.
- *
- * This is often the case for the primary plane of simple framebuffers. See
- * also drm_crtc_helper_atomic_check() for the respective CRTC-state check
- * helper function.
- *
- * RETURNS:
- * Zero on success, or an errno code otherwise.
- */
-int drm_plane_helper_atomic_check(struct drm_plane *plane, struct
drm_atomic_state *state)
-{
- struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state, plane);
- struct drm_crtc *new_crtc = new_plane_state->crtc;
- struct drm_crtc_state *new_crtc_state = NULL;
-
- if (new_crtc)
- new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
-
- return drm_atomic_helper_check_plane_state(new_plane_state,
new_crtc_state,
- DRM_PLANE_NO_SCALING,
- DRM_PLANE_NO_SCALING,
- false, false);
-}
-EXPORT_SYMBOL(drm_plane_helper_atomic_check);
diff --git a/drivers/gpu/drm/udl/udl_modeset.c
b/drivers/gpu/drm/udl/udl_modeset.c
index 40876bcdd79a4..7702359c90c22 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -21,7 +21,6 @@
#include
#include
#include
-#include
#include
#include
@@ -261,6 +260,22 @@ static const uint64_t udl_primary_plane_fmtmods[] = {
DRM_FORMAT_MOD_INVALID
};
+static int udl_primary_plane_helper_atomic_check(struct drm_plane *plane,
+struct drm_atomic_state *state)
+{
+ struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state, plane);
+ struct drm_crtc *new_crtc = new_plane_state->crtc;
+ struct drm_crtc_state *new_crtc_state = NULL;
+
+ if (new_crtc)
+ new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
+
+ return drm_atomic_helper_check_plane_state(new_plane_state,
new_crtc_state,
+ DRM_PLANE_NO_SCALING,
+ DRM_PLANE_NO_SCALING,
+ false, false);
+}
+
static void udl_primary_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_atomic_state
*state)
{
@@ -296,7 +311,7 @@ static void udl_primary_plane_helper_atomic_update(struct
drm_plane *plane,
static const struct drm_plane_helper_funcs udl_primary_plane_helper_funcs = {
DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
- .atomic_check = drm_plane_helper_atomic_check,
+ .atomic_check = udl_primary_plane_helper_atomic_check,
.atomic_update = udl_primary_plane_helper_atomic_update,
};
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 3a574e8cd22f4..75f9c4830564a 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -26,7 +26,6 @@
#include
-struct drm_atomic_state;
struct drm_crtc;
struct drm_fram
[PATCH v2 4/8] drm/shmobile: Do not include
Remove unnecessary include statements for . The file contains helpers for non-atomic code and should not be required by most drivers. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Geert Uytterhoeven Acked-by: Geert Uytterhoeven --- drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c index 8f9a728affde8..07ad17d24294d 100644 --- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c +++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c @@ -14,7 +14,6 @@ #include #include #include -#include #include "shmob_drm_drv.h" #include "shmob_drm_kms.h" -- 2.43.0
[PATCH v2 8/8] drm/xlnx: Do not include
Remove unnecessary include statements for . The file contains helpers for non-atomic code and should not be required by most drivers. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/xlnx/zynqmp_kms.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c index a7f8611be6f42..db3bb4afbfc46 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include -- 2.43.0
[PATCH v2 0/8] drm/plane-helpers: Minor clean ups
Move drm_plane_helper_atomic_check() into udl, which is the only driver using it. Remove several unnecessary include statements for . v2: * fix documentation (Sui) Thomas Zimmermann (8): drm/plane-helper: Move drm_plane_helper_atomic_check() into udl drm/amdgpu: Do not include drm/loongson: Do not include drm/shmobile: Do not include drm/solomon: Do not include drm/ofdrm: Do not include drm/simpledrm: Do not include drm/xlnx: Do not include .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 - drivers/gpu/drm/drm_crtc_helper.c | 7 ++-- drivers/gpu/drm/drm_plane_helper.c| 32 --- drivers/gpu/drm/loongson/lsdc_plane.c | 1 - .../drm/renesas/shmobile/shmob_drm_plane.c| 1 - drivers/gpu/drm/solomon/ssd130x.h | 1 - drivers/gpu/drm/tiny/ofdrm.c | 1 - drivers/gpu/drm/tiny/simpledrm.c | 1 - drivers/gpu/drm/udl/udl_modeset.c | 19 +-- drivers/gpu/drm/xlnx/zynqmp_kms.c | 1 - include/drm/drm_plane_helper.h| 2 -- 11 files changed, 19 insertions(+), 48 deletions(-) -- 2.43.0
[PATCH v2 3/8] drm/loongson: Do not include
Remove unnecessary include statements for . The file contains helpers for non-atomic code and should not be required by most drivers. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Sui Jingfeng Tested-by: Sui Jingfeng --- drivers/gpu/drm/loongson/lsdc_plane.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/loongson/lsdc_plane.c b/drivers/gpu/drm/loongson/lsdc_plane.c index 0d50946332229..d227a2c1dcf16 100644 --- a/drivers/gpu/drm/loongson/lsdc_plane.c +++ b/drivers/gpu/drm/loongson/lsdc_plane.c @@ -9,7 +9,6 @@ #include #include #include -#include #include "lsdc_drv.h" #include "lsdc_regs.h" -- 2.43.0
[PATCH v2 2/8] drm/amdgpu: Do not include
Remove unnecessary include statements for . The file contains helpers for non-atomic code and should not be required by most drivers. No functional changes. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index aa43f1761acd3..b8c3a9b104a41 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -92,7 +92,6 @@ #include #include #include -#include #include -- 2.43.0
[PATCH v2 7/8] drm/simpledrm: Do not include
Remove unnecessary include statements for . The file contains helpers for non-atomic code and should not be required by most drivers. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Sui Jingfeng --- drivers/gpu/drm/tiny/simpledrm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 34bbbd7b53dd9..7ce1c46176750 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -25,7 +25,6 @@ #include #include #include -#include #include #define DRIVER_NAME"simpledrm" -- 2.43.0
[PATCH v2 5/8] drm/solomon: Do not include
Remove unnecessary include statements for . The file contains helpers for non-atomic code and should not be required by most drivers. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Geert Uytterhoeven Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/solomon/ssd130x.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h index acf7cedf0c1ab..075c5c3ee75ac 100644 --- a/drivers/gpu/drm/solomon/ssd130x.h +++ b/drivers/gpu/drm/solomon/ssd130x.h @@ -17,7 +17,6 @@ #include #include #include -#include #include -- 2.43.0
[PATCH v2 6/8] drm/ofdrm: Do not include
Remove unnecessary include statements for . The file contains helpers for non-atomic code and should not be required by most drivers. No functional changes. Signed-off-by: Thomas Zimmermann Reviewed-by: Sui Jingfeng --- drivers/gpu/drm/tiny/ofdrm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c index 05a72473cfc65..ab89b7fc7bf61 100644 --- a/drivers/gpu/drm/tiny/ofdrm.c +++ b/drivers/gpu/drm/tiny/ofdrm.c @@ -19,7 +19,6 @@ #include #include #include -#include #include #include -- 2.43.0
Fwd: [PATCH 3/4] fbdev/vesafb: Replace references to global screen_info by local pointer
cc'ing mailing lists Weitergeleitete Nachricht Betreff: Re: [PATCH 3/4] fbdev/vesafb: Replace references to global screen_info by local pointer Datum: Fri, 01 Dec 2023 09:54:51 +0100 Von: Javier Martinez Canillas An: Thomas Zimmermann Thomas Zimmermann writes: Get the global screen_info's address once and access the data via this pointer. Limits the use of global state. Signed-off-by: Thomas Zimmermann --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH RESEND] drm/display: Drop obsolete dependency on COMPILE_TEST
Jean Delvare writes:
Hello Jean,
> Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
> is possible to test-build any driver which depends on OF on any
> architecture by explicitly selecting OF. Therefore depending on
> COMPILE_TEST as an alternative is no longer needed.
>
> Signed-off-by: Jean Delvare
> Cc: David Airlie
> Cc: Daniel Vetter
> ---
Reviewed-by: Javier Martinez Canillas
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH] drm/doc: Define KMS atomic state set
Hi On Fri, Dec 01, 2023 at 06:03:48PM +0200, Pekka Paalanen wrote: > On Fri, 1 Dec 2023 14:20:55 +0100 > Maxime Ripard wrote: > > > On Fri, Dec 01, 2023 at 12:06:48PM +0200, Pekka Paalanen wrote: > > > On Fri, 1 Dec 2023 10:25:09 +0100 > > > Maxime Ripard wrote: > > > > > > > On Fri, Dec 01, 2023 at 11:06:16AM +0200, Pekka Paalanen wrote: > > > > > On Fri, 1 Dec 2023 09:29:05 +0100 > > > > > Maxime Ripard wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > On Thu, Nov 30, 2023 at 05:07:40PM -0300, André Almeida wrote: > > > > > > > From: Pekka Paalanen > > > > > > > > > > > > > > Specify how the atomic state is maintained between userspace and > > > > > > > kernel, plus the special case for async flips. > > > > > > > > > > > > > > Signed-off-by: Pekka Paalanen > > > > > > > Signed-off-by: André Almeida > > > > > > > --- > > > > > > > > > > > > > > This is a standalone patch from the following serie, the other > > > > > > > patches are > > > > > > > already merged: > > > > > > > https://lore.kernel.org/lkml/20231122161941.320564-1-andrealm...@igalia.com/ > > > > > > > > > > > > > > Documentation/gpu/drm-uapi.rst | 47 > > > > > > > ++ > > > > > > > 1 file changed, 47 insertions(+) > > > > > > > > > > > > > > diff --git a/Documentation/gpu/drm-uapi.rst > > > > > > > b/Documentation/gpu/drm-uapi.rst > > > > > > > index 370d820be248..d0693f902a5c 100644 > > > > > > > --- a/Documentation/gpu/drm-uapi.rst > > > > > > > +++ b/Documentation/gpu/drm-uapi.rst > > > > > > > @@ -570,3 +570,50 @@ dma-buf interoperability > > > > > > > > > > > > > > Please see > > > > > > > Documentation/userspace-api/dma-buf-alloc-exchange.rst for > > > > > > > information on how dma-buf is integrated and exposed within DRM. > > > > > > > + > > > > > > > +KMS atomic state > > > > > > > + > > > > > > > + > > > > > > > +An atomic commit can change multiple KMS properties in an atomic > > > > > > > fashion, > > > > > > > +without ever applying intermediate or partial state changes. > > > > > > > Either the whole > > > > > > > +commit succeeds or fails, and it will never be applied > > > > > > > partially. This is the > > > > > > > +fundamental improvement of the atomic API over the older > > > > > > > non-atomic API which is > > > > > > > +referred to as the "legacy API". Applying intermediate state > > > > > > > could unexpectedly > > > > > > > +fail, cause visible glitches, or delay reaching the final state. > > > > > > > + > > > > > > > +An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, > > > > > > > which means the > > > > > > > +complete state change is validated but not applied. Userspace > > > > > > > should use this > > > > > > > +flag to validate any state change before asking to apply it. If > > > > > > > validation fails > > > > > > > +for any reason, userspace should attempt to fall back to > > > > > > > another, perhaps > > > > > > > +simpler, final state. This allows userspace to probe for > > > > > > > various configurations > > > > > > > +without causing visible glitches on screen and without the need > > > > > > > to undo a > > > > > > > +probing change. > > > > > > > + > > > > > > > +The changes recorded in an atomic commit apply on top the > > > > > > > current KMS state in > > > > > > > +the kernel. Hence, the complete new KMS state is the complete > > > > > > > old KMS state with > > > > > > > +the committed property settings done on top. The kernel will try > > > > > > > to avoid > > > > > > > > > > > > That part is pretty confusing to me. > > > > > > > > > > > > What are you calling the current and old KMS state? > > > > > > > > > > Current = old, if you read that "current" is the KMS state before > > > > > considering the atomic commit at hand. > > > > > > > > > > > What's confusing to me is that, yes, what you're saying is true for > > > > > > a > > > > > > given object: if it was part of the commit, the new state is the old > > > > > > state + whatever the new state changed. > > > > > > > > > > > > However, if that object wasn't part of the commit at all, then it's > > > > > > completely out of the old or new global KMS state. > > > > > > > > > > This is not talking about kernel data structures at all. This is > > > > > talking about how KMS looks from the userspace point of view. > > > > > > > > I mean, that's also true from the userspace point of view. You can very > > > > well commit only a single property on a single object, and only that > > > > object will be part of the "global KMS state". > > > > > > What is "global KMS state"? > > > > struct drm_atomic_state, ie. the object holding the entire new commit > > content. > > > > > As a userspace developer, the global KMS state is the complete, total, > > > hardware and driver instance state. It's not any kind of data > > > structure, but it is all the condition and all the programming of the > > > whol
[PATCH v2 0/4] fbdev: Remove global screen_info in efifb/vesafb
Replace the global instance of screen_info with the per-device instance that is set by the sysfb code. The use of the global screen_info should be limited and ideally be pushed into per-architecture code. v2: * comment on devm_kmemdup() usage (Javier) Thomas Zimmermann (4): fbdev/efifb: Replace references to global screen_info by local pointer fbdev/efifb: Use screen_info pointer from device fbdev/vesafb: Replace references to global screen_info by local pointer fbdev/vesafb: Use screen_info pointer from device drivers/video/fbdev/efifb.c | 125 --- drivers/video/fbdev/vesafb.c | 78 +- 2 files changed, 117 insertions(+), 86 deletions(-) -- 2.43.0
[PATCH v2 2/4] fbdev/efifb: Use screen_info pointer from device
Use the screen_info instance from the device instead of dereferencing
the global screen_info state. Decouples the driver from per-architecture
code. Duplicated the screen_info data, so that efifb can modify it at
will.
v2:
* comment on devm_kmemdup() usage (Javier)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Javier Martinez Canillas
---
drivers/video/fbdev/efifb.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 6cbb65bbe1110..4d7e899a1c853 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -358,7 +358,7 @@ static u64 bar_offset;
static int efifb_probe(struct platform_device *dev)
{
- struct screen_info *si = &screen_info;
+ struct screen_info *si;
struct fb_info *info;
struct efifb_par *par;
int err, orientation;
@@ -368,6 +368,18 @@ static int efifb_probe(struct platform_device *dev)
char *option = NULL;
efi_memory_desc_t md;
+ /*
+* If we fail probing the device, the kernel might try a different
+* driver. We get a copy of the attached screen_info, so that we can
+* modify its values without affecting later drivers.
+*/
+ si = dev_get_platdata(&dev->dev);
+ if (!si)
+ return -ENODEV;
+ si = devm_kmemdup(&dev->dev, si, sizeof(*si), GFP_KERNEL);
+ if (!si)
+ return -ENOMEM;
+
if (si->orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
return -ENODEV;
--
2.43.0
[PATCH v2 1/4] fbdev/efifb: Replace references to global screen_info by local pointer
Get the global screen_info's address once and access the data via
this pointer. Limits the use of global state.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Javier Martinez Canillas
---
drivers/video/fbdev/efifb.c | 113 ++--
1 file changed, 58 insertions(+), 55 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index f9b4ddd592ce4..6cbb65bbe1110 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -147,10 +147,9 @@ static bool efifb_bgrt_sanity_check(struct screen_info
*si, u32 bmp_width)
}
#endif
-static void efifb_show_boot_graphics(struct fb_info *info)
+static void efifb_show_boot_graphics(struct fb_info *info, struct screen_info
*si)
{
u32 bmp_width, bmp_height, bmp_pitch, dst_x, y, src_y;
- struct screen_info *si = &screen_info;
struct bmp_file_header *file_header;
struct bmp_dib_header *dib_header;
void *bgrt_image = NULL;
@@ -282,7 +281,7 @@ static const struct fb_ops efifb_ops = {
.fb_setcolreg = efifb_setcolreg,
};
-static int efifb_setup(char *options)
+static int efifb_setup(struct screen_info *si, char *options)
{
char *this_opt;
@@ -290,16 +289,16 @@ static int efifb_setup(char *options)
while ((this_opt = strsep(&options, ",")) != NULL) {
if (!*this_opt) continue;
- efifb_setup_from_dmi(&screen_info, this_opt);
+ efifb_setup_from_dmi(si, this_opt);
if (!strncmp(this_opt, "base:", 5))
- screen_info.lfb_base =
simple_strtoul(this_opt+5, NULL, 0);
+ si->lfb_base = simple_strtoul(this_opt+5, NULL,
0);
else if (!strncmp(this_opt, "stride:", 7))
- screen_info.lfb_linelength =
simple_strtoul(this_opt+7, NULL, 0) * 4;
+ si->lfb_linelength = simple_strtoul(this_opt+7,
NULL, 0) * 4;
else if (!strncmp(this_opt, "height:", 7))
- screen_info.lfb_height =
simple_strtoul(this_opt+7, NULL, 0);
+ si->lfb_height = simple_strtoul(this_opt+7,
NULL, 0);
else if (!strncmp(this_opt, "width:", 6))
- screen_info.lfb_width =
simple_strtoul(this_opt+6, NULL, 0);
+ si->lfb_width = simple_strtoul(this_opt+6,
NULL, 0);
else if (!strcmp(this_opt, "nowc"))
mem_flags &= ~EFI_MEMORY_WC;
else if (!strcmp(this_opt, "nobgrt"))
@@ -310,15 +309,15 @@ static int efifb_setup(char *options)
return 0;
}
-static inline bool fb_base_is_valid(void)
+static inline bool fb_base_is_valid(struct screen_info *si)
{
- if (screen_info.lfb_base)
+ if (si->lfb_base)
return true;
- if (!(screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE))
+ if (!(si->capabilities & VIDEO_CAPABILITY_64BIT_BASE))
return false;
- if (screen_info.ext_lfb_base)
+ if (si->ext_lfb_base)
return true;
return false;
@@ -329,7 +328,10 @@ static ssize_t name##_show(struct device *dev,
\
struct device_attribute *attr, \
char *buf) \
{ \
- return sprintf(buf, fmt "\n", (screen_info.lfb_##name));\
+ struct screen_info *si = dev_get_platdata(dev); \
+ if (!si)\
+ return -ENODEV; \
+ return sprintf(buf, fmt "\n", (si->lfb_##name));\
} \
static DEVICE_ATTR_RO(name)
@@ -356,6 +358,7 @@ static u64 bar_offset;
static int efifb_probe(struct platform_device *dev)
{
+ struct screen_info *si = &screen_info;
struct fb_info *info;
struct efifb_par *par;
int err, orientation;
@@ -365,48 +368,48 @@ static int efifb_probe(struct platform_device *dev)
char *option = NULL;
efi_memory_desc_t md;
- if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
+ if (si->orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
return -ENODEV;
if (fb_get_options("efifb", &option))
return -ENODEV;
- efifb_setup(option);
+ efifb_setup(si, option);
/* We don't get linelength from UGA Draw Protocol, only from
* EFI Graphics Protocol. So if it's not in DMI, and it's not
* passed in from the user, we really can't use the framebuffer.
[PATCH v2 4/4] fbdev/vesafb: Use screen_info pointer from device
Use the screen_info instance from the device instead of dereferencing
the global screen_info state. Decouples the driver from per-architecture
code. Duplicated the screen_info data, so that vesafb can modify it at
will.
v2:
* comment on devm_kmemdup() usage (Javier)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Javier Martinez Canillas
---
drivers/video/fbdev/vesafb.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index ea89accbec385..8ab64ae4cad3e 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -243,7 +243,7 @@ static int vesafb_setup(char *options)
static int vesafb_probe(struct platform_device *dev)
{
- struct screen_info *si = &screen_info;
+ struct screen_info *si;
struct fb_info *info;
struct vesafb_par *par;
int i, err;
@@ -252,6 +252,18 @@ static int vesafb_probe(struct platform_device *dev)
unsigned int size_total;
char *option = NULL;
+ /*
+* If we fail probing the device, the kernel might try a different
+* driver. We get a copy of the attached screen_info, so that we can
+* modify its values without affecting later drivers.
+*/
+ si = dev_get_platdata(&dev->dev);
+ if (!si)
+ return -ENODEV;
+ si = devm_kmemdup(&dev->dev, si, sizeof(*si), GFP_KERNEL);
+ if (!si)
+ return -ENOMEM;
+
/* ignore error return of fb_get_options */
fb_get_options("vesafb", &option);
vesafb_setup(option);
--
2.43.0
[PATCH v2 3/4] fbdev/vesafb: Replace references to global screen_info by local pointer
Get the global screen_info's address once and access the data via
this pointer. Limits the use of global state.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Javier Martinez Canillas
---
drivers/video/fbdev/vesafb.c | 66 +++-
1 file changed, 35 insertions(+), 31 deletions(-)
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index c0edceea0a793..ea89accbec385 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -243,6 +243,7 @@ static int vesafb_setup(char *options)
static int vesafb_probe(struct platform_device *dev)
{
+ struct screen_info *si = &screen_info;
struct fb_info *info;
struct vesafb_par *par;
int i, err;
@@ -255,17 +256,17 @@ static int vesafb_probe(struct platform_device *dev)
fb_get_options("vesafb", &option);
vesafb_setup(option);
- if (screen_info.orig_video_isVGA != VIDEO_TYPE_VLFB)
+ if (si->orig_video_isVGA != VIDEO_TYPE_VLFB)
return -ENODEV;
- vga_compat = (screen_info.capabilities & 2) ? 0 : 1;
- vesafb_fix.smem_start = screen_info.lfb_base;
- vesafb_defined.bits_per_pixel = screen_info.lfb_depth;
+ vga_compat = (si->capabilities & 2) ? 0 : 1;
+ vesafb_fix.smem_start = si->lfb_base;
+ vesafb_defined.bits_per_pixel = si->lfb_depth;
if (15 == vesafb_defined.bits_per_pixel)
vesafb_defined.bits_per_pixel = 16;
- vesafb_defined.xres = screen_info.lfb_width;
- vesafb_defined.yres = screen_info.lfb_height;
- vesafb_fix.line_length = screen_info.lfb_linelength;
+ vesafb_defined.xres = si->lfb_width;
+ vesafb_defined.yres = si->lfb_height;
+ vesafb_fix.line_length = si->lfb_linelength;
vesafb_fix.visual = (vesafb_defined.bits_per_pixel == 8) ?
FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;
@@ -277,7 +278,7 @@ static int vesafb_probe(struct platform_device *dev)
/* size_total -- all video memory we have. Used for mtrr
* entries, resource allocation and bounds
* checking. */
- size_total = screen_info.lfb_size * 65536;
+ size_total = si->lfb_size * 65536;
if (vram_total)
size_total = vram_total * 1024 * 1024;
if (size_total < size_vmode)
@@ -297,7 +298,7 @@ static int vesafb_probe(struct platform_device *dev)
vesafb_fix.smem_len = size_remap;
#ifndef __i386__
- screen_info.vesapm_seg = 0;
+ si->vesapm_seg = 0;
#endif
if (!request_mem_region(vesafb_fix.smem_start, size_total, "vesafb")) {
@@ -317,23 +318,26 @@ static int vesafb_probe(struct platform_device *dev)
par = info->par;
info->pseudo_palette = par->pseudo_palette;
- par->base = screen_info.lfb_base;
+ par->base = si->lfb_base;
par->size = size_total;
printk(KERN_INFO "vesafb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
- vesafb_defined.xres, vesafb_defined.yres,
vesafb_defined.bits_per_pixel, vesafb_fix.line_length, screen_info.pages);
+ vesafb_defined.xres, vesafb_defined.yres,
vesafb_defined.bits_per_pixel,
+ vesafb_fix.line_length, si->pages);
- if (screen_info.vesapm_seg) {
+ if (si->vesapm_seg) {
printk(KERN_INFO "vesafb: protected mode interface info at
%04x:%04x\n",
- screen_info.vesapm_seg,screen_info.vesapm_off);
+ si->vesapm_seg, si->vesapm_off);
}
- if (screen_info.vesapm_seg < 0xc000)
+ if (si->vesapm_seg < 0xc000)
ypan = pmi_setpal = 0; /* not available or some DOS TSR ... */
if (ypan || pmi_setpal) {
+ unsigned long pmi_phys;
unsigned short *pmi_base;
- pmi_base = (unsigned short*)phys_to_virt(((unsigned
long)screen_info.vesapm_seg << 4) + screen_info.vesapm_off);
+ pmi_phys = ((unsigned long)si->vesapm_seg << 4) +
si->vesapm_off;
+ pmi_base = (unsigned short *)phys_to_virt(pmi_phys);
pmi_start = (void*)((char*)pmi_base + pmi_base[1]);
pmi_pal = (void*)((char*)pmi_base + pmi_base[2]);
printk(KERN_INFO "vesafb: pmi: set display start = %p, set
palette = %p\n",pmi_start,pmi_pal);
@@ -377,14 +381,14 @@ static int vesafb_probe(struct platform_device *dev)
vesafb_defined.left_margin = (vesafb_defined.xres / 8) & 0xf8;
vesafb_defined.hsync_len= (vesafb_defined.xres / 8) & 0xf8;
- vesafb_defined.red.offset= screen_info.red_pos;
- vesafb_defined.red.length= screen_info.red_size;
- vesafb_defined.green.offset = screen_info.green_pos;
- vesafb_defined.green.length = screen_info.green_size;
- vesafb_defined.blue.offset = screen_info.blue_pos;
- vesafb_defined.blue.length = screen_info.blue_size;
- vesafb
Re: [PATCH v2 05/10] drm/bridge: tc358775: make standby GPIO optional
The stby pin is optional. It is only needed for power-up and down sequencing. It is not needed, if the power rails cannot by dynamically enabled. Because the GPIO is not optional, remove the error message. I just noticed a typo: "is *now* optional. -michael
Re: [PATCH v2 01/10] dt-bindings: display: bridge: tc358775: make stby gpio and vdd supplies optional
For a normal operation, the vdd supplies nor the stby GPIO is needed. There are boards, where these voltages are statically enabled during board power-up. This means supply is still required. You mean using fixed-regulator? I didn't consider that. But yes, you're right. -michael
Re: [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices
Am 04.12.23 um 00:32 schrieb Alistair Popple: Christian König writes: Am 01.12.23 um 06:48 schrieb Zeng, Oak: [SNIP] Besides memory eviction/oversubscription, there are a few other pain points when I use hmm: 1) hmm doesn't support file-back memory, so it is hard to share memory b/t process in a gpu environment. You mentioned you have a plan... How hard is it to support file-backed in your approach? As hard as it is to support it through HMM. That's what I meant that this approach doesn't integrate well, as far as I know the problem isn't inside HMM or any other solution but rather in the file system layer. In what way does HMM not support file-backed memory? I was under the impression that at least hmm_range_fault() does. Oh, well file-backed memory is indeed supported by HMM. IIRC KFD actually allows this for the SVM implementation. It's just that the way the file system layer (for example) does writeback absolutely doesn't fit well with how GPUs and other acceleration devices work. The general assumption in the kernel seems to be that page faults and preemption are extremely cheap. So things like copy on write is used quite extensively. For a CPU this basically means you just need to context change into the kernel once to get the new address of a page into your PTEs on write, while for acceleration devices this always require a complete CPU round trip for each initial write access for a 4k page. The performance impact is just horrible. Regards, Christian. - Alistair Regards, Christian. 2)virtual address range based memory attribute/hint: with hmadvise, where do you save the memory attribute of a virtual address range? Do you need to extend vm_area_struct to save it? With hmm, we have to maintain such information at driver. This ends up with pretty complicated logic to split/merge those address range. I know core mm has similar logic to split/merge vma... Oak -Weixi -Original Message- From: Christian König Sent: Thursday, November 30, 2023 4:28 PM To: Zeng, Oak; Christian König ; zhuweixi; linux- m...@kvack.org;linux-ker...@vger.kernel.org;a...@linux-foundation.org; Danilo Krummrich; Dave Airlie; Daniel Vetter Cc:intel-gvt-...@lists.freedesktop.org;rcampb...@nvidia.com; mhairgr...@nvidia.com;j...@nvidia.com;weixi@openeuler.sh; jhubb...@nvidia.com;intel-...@lists.freedesktop.org;apop...@nvidia.com; xinhui@amd.com;amd-...@lists.freedesktop.org; tvrtko.ursu...@linux.intel.com;ogab...@kernel.org;jgli...@redhat.com; dri- de...@lists.freedesktop.org;z...@nvidia.com; Vivi, Rodrigo ;alexander.deuc...@amd.com;leo...@nvidia.com; felix.kuehl...@amd.com; Wang, Zhi A; mgor...@suse.de Subject: Re: [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices Hi Oak, yeah, #4 is indeed a really good point and I think Felix will agree to that as well. HMM is basically still missing a way to advise device attributes for the CPU address space. Both migration strategy as well as device specific information (like cache preferences) fall into this category. Since there is a device specific component in those attributes as well I think device specific IOCTLs still make sense to update them, but HMM should offer the functionality to manage and store those information. Split and merge of VMAs only become a problem if you attach those information to VMAs, if you keep them completely separate than that doesn't become an issue either. The down side of this approach is that you don't get automatically extending attribute ranges for growing VMAs for example. Regards, Christian. Am 29.11.23 um 23:23 schrieb Zeng, Oak: Hi Weixi, Even though Christian has listed reasons rejecting this proposal (yes they are very reasonable to me), I would open my mind and further explore the possibility here. Since the current GPU driver uses a hmm based implementation (AMD and NV has done this; At Intel we are catching up), I want to explore how much we can benefit from the proposed approach and how your approach can solve some pain points of our development. So basically what I am questioning here is: what is the advantage of your approach against hmm. To implement a UVM (unified virtual address space b/t cpu and gpu device), with hmm, driver essentially need to implement below functions: 1. device page table update. Your approach requires the same because this is device specific codes 2. Some migration functions to migrate memory b/t system memory and GPU local memory. My understanding is, even though you generalized this a bit, such as modified cpu page fault path, provided "general" gm_dev_fault handler... but device driver still need to provide migration functions because migration functions have to be device specific (i.e., using device dma/copy engine for performance purpose). Right? 3. GPU physical memory management, this part is now in drm/buddy, shared by all drivers. I think with your approach, driver still need to provide cal
Re: [RFC PATCH] of/platform: Disable sysfb if a simple-framebuffer node is found
Rob Herring writes: Hello Rob, > On Fri, Dec 1, 2023 at 4:21 AM Javier Martinez Canillas [...] >> >> I don't have a better idea than this patch but maybe Thomas or Sima do? >> > >> > At SUSE, we've carried such a patch in our repos for some time. It works >> > around the double-framebuffer problem in a similar way. [1] >> > >> >> Thanks for the information. I see that your patch is basically mine but >> doing it unconditionally while this one only does the sysfb_disable() if >> a "simple-framebuffer" DT node has been found. >> >> > As Javier mentioned, we do track the framebuffer ranges for EFI/VESA/OF >> > framebuffers in the graphics aperture helpers. Fbdev has done this for >> > decades, and the current codebase extends and harmonizes this >> > functionality among fbdev and DRM drivers. >> > >> > WRT DT vs EFI: AFAIK the EFI support on affected platforms looks at the >> > DT to set up the EFI framebuffer. So IMHO the DT is the authoritative >> > source on the framebuffer. >> > >> >> Agreed. Sima Vetter also mentioned on IRC that they think this patch is >> the least bad option. Rob, Ard any thoughts? Maybe we can land this as >> a fix and then figure how this could be better handled in the long term? > > What about moving the DT setup code here to sysfb? Then we just setup > the right thing up front. > Right now sysfb is pretty platform agnostic, in the sense that just looks at the screen_info global struct and uses it to add the platform data that contains the firmware provided framebuffer. How the screen_info was filled (e.g: by the Linux EFI stub using the GOP or the x86 early boot code using VESA) is transparent to sysfb. For this reason adding platform specific logic there, like finding OF nodes, is not desirable. I also suggested to Thomas to move the DT setup code to sysfb but he said that would be the wrong approach for the reason mentioned above. Please correct me Thomas if I'm misremembering here. > However, there might be one other issue with that and this fix. The DT > simplefb can have resources such as clocks and regulators. With > fw_devlink, the driver won't probe until those dependencies are met. > So if you want the framebuffer console up early, then you may want to > register the EFI framebuffer first and then handoff to the DT simplefb > when it probes (rather than registering the device). > > But I agree, probably better to take this patch now and have those > quirks instead of flat out not working. > If we do that what's the plan? Are you thinking about merging this patch through your OF tree or do you want to go through drm-misc with your ack? > Rob > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v2 7/8] drm/simpledrm: Do not include
Thomas Zimmermann writes: > Remove unnecessary include statements for . > The file contains helpers for non-atomic code and should not be > required by most drivers. No functional changes. > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Sui Jingfeng > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH] drm/i915/dsi: Use devm_gpiod_get() for all GPIOs
On Fri, 01 Dec 2023, Hans de Goede wrote:
> soc_gpio_set_value() already uses devm_gpiod_get(), lets be consistent
> and use devm_gpiod_get() for all GPIOs.
>
> This allows removing the intel_dsi_vbt_gpio_cleanup() function,
> which only function was to put the GPIO-descriptors.
>
> Signed-off-by: Hans de Goede
Acked-by: Jani Nikula
> ---
> Note this applies on top of Andy's recent GPIO rework series which
> has just landed in drm-intel-next
> ---
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 17 ++---
> drivers/gpu/drm/i915/display/intel_dsi_vbt.h | 1 -
> drivers/gpu/drm/i915/display/vlv_dsi.c | 10 +-
> 3 files changed, 3 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index 275d0218394c..a5d7fc8418c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -922,7 +922,7 @@ void intel_dsi_vbt_gpio_init(struct intel_dsi *intel_dsi,
> bool panel_is_on)
> gpiod_add_lookup_table(gpiod_lookup_table);
>
> if (want_panel_gpio) {
> - intel_dsi->gpio_panel = gpiod_get(dev->dev, "panel", flags);
> + intel_dsi->gpio_panel = devm_gpiod_get(dev->dev, "panel",
> flags);
> if (IS_ERR(intel_dsi->gpio_panel)) {
> drm_err(&dev_priv->drm,
> "Failed to own gpio for panel control\n");
> @@ -932,7 +932,7 @@ void intel_dsi_vbt_gpio_init(struct intel_dsi *intel_dsi,
> bool panel_is_on)
>
> if (want_backlight_gpio) {
> intel_dsi->gpio_backlight =
> - gpiod_get(dev->dev, "backlight", flags);
> + devm_gpiod_get(dev->dev, "backlight", flags);
> if (IS_ERR(intel_dsi->gpio_backlight)) {
> drm_err(&dev_priv->drm,
> "Failed to own gpio for backlight control\n");
> @@ -943,16 +943,3 @@ void intel_dsi_vbt_gpio_init(struct intel_dsi
> *intel_dsi, bool panel_is_on)
> if (gpiod_lookup_table)
> gpiod_remove_lookup_table(gpiod_lookup_table);
> }
> -
> -void intel_dsi_vbt_gpio_cleanup(struct intel_dsi *intel_dsi)
> -{
> - if (intel_dsi->gpio_panel) {
> - gpiod_put(intel_dsi->gpio_panel);
> - intel_dsi->gpio_panel = NULL;
> - }
> -
> - if (intel_dsi->gpio_backlight) {
> - gpiod_put(intel_dsi->gpio_backlight);
> - intel_dsi->gpio_backlight = NULL;
> - }
> -}
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
> index 468d873fab1a..3462fcc760e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.h
> @@ -13,7 +13,6 @@ struct intel_dsi;
>
> bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
> void intel_dsi_vbt_gpio_init(struct intel_dsi *intel_dsi, bool panel_is_on);
> -void intel_dsi_vbt_gpio_cleanup(struct intel_dsi *intel_dsi);
> void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
>enum mipi_seq seq_id);
> void intel_dsi_log_params(struct intel_dsi *intel_dsi);
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c
> b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 5bda97c96810..9b33b8a74d64 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1532,16 +1532,8 @@ static void intel_dsi_unprepare(struct intel_encoder
> *encoder)
> }
> }
>
> -static void intel_dsi_encoder_destroy(struct drm_encoder *encoder)
> -{
> - struct intel_dsi *intel_dsi =
> enc_to_intel_dsi(to_intel_encoder(encoder));
> -
> - intel_dsi_vbt_gpio_cleanup(intel_dsi);
> - intel_encoder_destroy(encoder);
> -}
> -
> static const struct drm_encoder_funcs intel_dsi_funcs = {
> - .destroy = intel_dsi_encoder_destroy,
> + .destroy = intel_encoder_destroy,
> };
>
> static enum drm_mode_status vlv_dsi_mode_valid(struct drm_connector
> *connector,
--
Jani Nikula, Intel
Re: [PATCH v2 09/10] drm/bridge: tc358775: Add support for tc358765
>> The tc358775 bridge is pin compatible with earlier tc358765 according to
>> the tc358774xbg_datasheet_en_20190118.pdf documentation. Compared to the
>> tc358765, the tc358775 supports a STBY GPIO and higher data rates.
>>
>> The tc358765 has a register bit for video event mode vs video pulse mode.
>> We must set it to video event mode for the LCD output to work, and on the
>> tc358775, this bit no longer exists.
>>
>> Looks like the registers seem to match otherwise based on a quick glance
>> comparing the defines to the earlier Android kernel tc358765 driver.
>>
>> Signed-off-by: Tony Lindgren
>> ---
>> drivers/gpu/drm/bridge/tc358775.c | 26 ++
>> 1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358775.c
>> b/drivers/gpu/drm/bridge/tc358775.c
>> --- a/drivers/gpu/drm/bridge/tc358775.c
>> +++ b/drivers/gpu/drm/bridge/tc358775.c
>> @@ -15,6 +15,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -107,6 +108,7 @@
>> #define RDPKTLN 0x0404 /* Command Read Packet Length */
>>
>> #define VPCTRL 0x0450 /* Video Path Control */
>> +#define EVTMODEBIT(5) /* Video event mode enable, tc35876x
>> only */
>> #define HTIM1 0x0454 /* Horizontal Timing Control 1 */
>> #define HTIM2 0x0458 /* Horizontal Timing Control 2 */
>> #define VTIM1 0x045C /* Vertical Timing Control 1 */
>> @@ -254,6 +256,11 @@ enum tc358775_ports {
>> TC358775_LVDS_OUT1,
>> };
>>
>> +enum tc3587x5_type {
>> + TC358765,
>
> I'd suggest adding = 1 or =0x65 so that the specified type differs
> from 'no data' = 0 / NULL.
>
>> + TC358775,
>> +};
>> +
>> struct tc_data {
>> struct i2c_client *i2c;
>> struct device *dev;
>> @@ -271,6 +278,8 @@ struct tc_data {
>> struct gpio_desc*stby_gpio;
>> u8 lvds_link; /* single-link or dual-link */
>> u8 bpc;
>> +
>> + enum tc3587x5_type type;
>> };
>>
>> static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
>> @@ -424,10 +433,16 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
>> d2l_write(tc->i2c, PPI_STARTPPI, PPI_START_FUNCTION);
>> d2l_write(tc->i2c, DSI_STARTDSI, DSI_RX_START);
>>
>> + /* Video event mode vs pulse mode bit, does not exist for tc358775 */
>> + if (tc->type == TC358765)
>> + val = EVTMODE;
>> + else
>> + val = 0;
>> +
>> if (tc->bpc == 8)
>> - val = TC358775_VPCTRL_OPXLFMT(1);
>> + val |= TC358775_VPCTRL_OPXLFMT(1);
>> else /* bpc = 6; */
>> - val = TC358775_VPCTRL_MSF(1);
>> + val |= TC358775_VPCTRL_MSF(1);
>>
>> dsiclk = mode->crtc_clock * 3 * tc->bpc / tc->num_dsi_lanes / 1000;
>> clkdiv = dsiclk / (tc->lvds_link == DUAL_LINK ? DIVIDE_BY_6 :
>> DIVIDE_BY_3);
>> @@ -643,6 +658,7 @@ static int tc_probe(struct i2c_client *client)
>>
>> tc->dev = dev;
>> tc->i2c = client;
>> + tc->type = (enum tc3587x5_type)of_device_get_match_data(dev);
>
> Would it make sense to use i2c_get_match_data() instead?
FWIW, I' planning to add a dsi binding for this driver. So I'd
suggest either the of_ or the device_ variant. Not sure though,
if the new device supports the DSI commands.
Otherwise, the patch looks good:
Reviewed-by: Michael Walle
-michael
>
>>
>> tc->panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node,
>> TC358775_LVDS_OUT0, 0);
>> @@ -704,13 +720,15 @@ static void tc_remove(struct i2c_client *client)
>> }
>>
>> static const struct i2c_device_id tc358775_i2c_ids[] = {
>> - { "tc358775", 0 },
>> + { "tc358765", TC358765, },
>> + { "tc358775", TC358775, },
>> { }
>> };
>> MODULE_DEVICE_TABLE(i2c, tc358775_i2c_ids);
>>
>> static const struct of_device_id tc358775_of_ids[] = {
>> - { .compatible = "toshiba,tc358775", },
>> + { .compatible = "toshiba,tc358765", .data = (void *)TC358765, },
>> + { .compatible = "toshiba,tc358775", .data = (void *)TC358775, },
>> { }
>> };
>> MODULE_DEVICE_TABLE(of, tc358775_of_ids);
>> --
>> 2.43.0
--
With best wishes
Dmitry
Re: [PATCH] drm/bridge: tc358768: select CONFIG_VIDEOMODE_HELPERS
On 04/12/2023 08:27, Arnd Bergmann wrote:
From: Arnd Bergmann
A dependency on this feature was recently introduced:
x86_64-linux-ld: vmlinux.o: in function `tc358768_bridge_pre_enable':
tc358768.c:(.text+0xbe3dae): undefined reference to
`drm_display_mode_to_videomode'
Make sure this is always enabled.
Fixes: e5fb21678136 ("drm/bridge: tc358768: Use struct videomode")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/bridge/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index ba82a1142adf..3e6a4e2044c0 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -313,6 +313,7 @@ config DRM_TOSHIBA_TC358768
select REGMAP_I2C
select DRM_PANEL
select DRM_MIPI_DSI
+ select VIDEOMODE_HELPERS
help
Toshiba TC358768AXBG/TC358778XBG DSI bridge chip driver.
Reviewed-by: Neil Armstrong
Re: fbcon on one of multiple displays
Eric Nelson writes: Hello Eric, > Hi all, > > Is there a way to configure a framebuffer console on a specific display > on a machine with multiple displays? > Have you looked at https://www.kernel.org/doc/Documentation/fb/fbcon.txt ? There is a sysfb interface to bind / unbind fbdev devices to fbcon's VT (/sys/class/vtconsole/vtcon1/bind) and also kernel command line parameter to choose which fbdev driver is mapped to which VT console (fbcon=map:n). -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
[PATCH v2 1/4] drm/stm: dsi: use new SYSTEM_SLEEP_PM_OPS() macro
Use RUNTIME_PM_OPS() instead of the old SET_SYSTEM_SLEEP_PM_OPS().
This means we don't need __maybe_unused on the functions.
Signed-off-by: Raphael Gallais-Pou
---
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index d5f8c923d7bc..b1aee43d51e9 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -544,7 +544,7 @@ static void dw_mipi_dsi_stm_remove(struct platform_device
*pdev)
regulator_disable(dsi->vdd_supply);
}
-static int __maybe_unused dw_mipi_dsi_stm_suspend(struct device *dev)
+static int dw_mipi_dsi_stm_suspend(struct device *dev)
{
struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
@@ -556,7 +556,7 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct
device *dev)
return 0;
}
-static int __maybe_unused dw_mipi_dsi_stm_resume(struct device *dev)
+static int dw_mipi_dsi_stm_resume(struct device *dev)
{
struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
int ret;
@@ -580,8 +580,8 @@ static int __maybe_unused dw_mipi_dsi_stm_resume(struct
device *dev)
}
static const struct dev_pm_ops dw_mipi_dsi_stm_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(dw_mipi_dsi_stm_suspend,
- dw_mipi_dsi_stm_resume)
+ SYSTEM_SLEEP_PM_OPS(dw_mipi_dsi_stm_suspend,
+ dw_mipi_dsi_stm_resume)
};
static struct platform_driver dw_mipi_dsi_stm_driver = {
--
2.25.1
[PATCH v2 0/4] Update STM DSI PHY driver
This patch series aims to add several features of the dw-mipi-dsi phy driver that are missing or need to be updated. First patch update a PM macro. Second patch adds runtime PM functionality to the driver. Third patch adds a clock provider generated by the PHY itself. As explained in the commit log of the second patch, a clock declaration is missing. Since this clock is parent of 'dsi_k', it leads to an orphan clock. Most importantly this patch is an anticipation for future versions of the DSI PHY, and its inclusion within the display subsystem and the DRM framework. Last patch fixes a corner effect introduced previously. Since 'dsi' and 'dsi_k' are gated by the same bit on the same register, both reference work as peripheral clock in the device-tree. --- Changes in v2: - Added patch 1/4 to use SYSTEM_SLEEP_PM_OPS instead of old macro and removed __maybe_used for accordingly - Changed SET_RUNTIME_PM_OPS to RUNTIME_PM_OPS Raphael Gallais-Pou (3): drm/stm: dsi: use new SYSTEM_SLEEP_PM_OPS() macro drm/stm: dsi: expose DSI PHY internal clock arm: dts: st: fix DSI peripheral clock on stm32mp15 boards Yannick Fertre (1): drm/stm: dsi: add pm runtime ops arch/arm/boot/dts/st/stm32mp157.dtsi | 2 +- arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts | 2 +- arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts | 2 +- arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts | 2 +- arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts | 2 +- drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 278 +++--- 6 files changed, 242 insertions(+), 46 deletions(-) -- 2.25.1
[PATCH v2 2/4] drm/stm: dsi: add pm runtime ops
From: Yannick Fertre
Update control of clocks and supply thanks to the PM runtime
mechanism to avoid kernel crash during a system suspend.
Signed-off-by: Yannick Fertre
Signed-off-by: Raphael Gallais-Pou
---
Changes in v2:
- Changed SET_RUNTIME_PM_OPS to RUNTIME_PM_OPS and removed
__maybe_unused
---
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index b1aee43d51e9..82fff9e84345 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
@@ -77,6 +78,7 @@ enum dsi_color {
struct dw_mipi_dsi_stm {
void __iomem *base;
struct clk *pllref_clk;
+ struct clk *pclk;
struct dw_mipi_dsi *dsi;
u32 hw_version;
int lane_min_kbps;
@@ -443,7 +445,6 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
*pdev)
{
struct device *dev = &pdev->dev;
struct dw_mipi_dsi_stm *dsi;
- struct clk *pclk;
int ret;
dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
@@ -483,21 +484,21 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
*pdev)
goto err_clk_get;
}
- pclk = devm_clk_get(dev, "pclk");
- if (IS_ERR(pclk)) {
- ret = PTR_ERR(pclk);
+ dsi->pclk = devm_clk_get(dev, "pclk");
+ if (IS_ERR(dsi->pclk)) {
+ ret = PTR_ERR(dsi->pclk);
DRM_ERROR("Unable to get peripheral clock: %d\n", ret);
goto err_dsi_probe;
}
- ret = clk_prepare_enable(pclk);
+ ret = clk_prepare_enable(dsi->pclk);
if (ret) {
DRM_ERROR("%s: Failed to enable peripheral clk\n", __func__);
goto err_dsi_probe;
}
dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
- clk_disable_unprepare(pclk);
+ clk_disable_unprepare(dsi->pclk);
if (dsi->hw_version != HWVER_130 && dsi->hw_version != HWVER_131) {
ret = -ENODEV;
@@ -551,6 +552,7 @@ static int dw_mipi_dsi_stm_suspend(struct device *dev)
DRM_DEBUG_DRIVER("\n");
clk_disable_unprepare(dsi->pllref_clk);
+ clk_disable_unprepare(dsi->pclk);
regulator_disable(dsi->vdd_supply);
return 0;
@@ -569,8 +571,16 @@ static int dw_mipi_dsi_stm_resume(struct device *dev)
return ret;
}
+ ret = clk_prepare_enable(dsi->pclk);
+ if (ret) {
+ regulator_disable(dsi->vdd_supply);
+ DRM_ERROR("Failed to enable pclk: %d\n", ret);
+ return ret;
+ }
+
ret = clk_prepare_enable(dsi->pllref_clk);
if (ret) {
+ clk_disable_unprepare(dsi->pclk);
regulator_disable(dsi->vdd_supply);
DRM_ERROR("Failed to enable pllref_clk: %d\n", ret);
return ret;
@@ -582,6 +592,8 @@ static int dw_mipi_dsi_stm_resume(struct device *dev)
static const struct dev_pm_ops dw_mipi_dsi_stm_pm_ops = {
SYSTEM_SLEEP_PM_OPS(dw_mipi_dsi_stm_suspend,
dw_mipi_dsi_stm_resume)
+ RUNTIME_PM_OPS(dw_mipi_dsi_stm_suspend,
+ dw_mipi_dsi_stm_resume, NULL)
};
static struct platform_driver dw_mipi_dsi_stm_driver = {
--
2.25.1
[PATCH v2 3/4] drm/stm: dsi: expose DSI PHY internal clock
DSISRC __
__\_
|\
pll4_p_ck ->| 1 |dsi_k
ck_dsi_phy ->| 0 |
|/
A DSI clock is missing in the clock framework. Looking at the
clk_summary, it appears that 'ck_dsi_phy' is not implemented. Since the
DSI kernel clock is based on the internal DSI pll. The common clock
driver can not directly expose this 'ck_dsi_phy' clock because it does
not contain any common registers with the DSI. Thus it needs to be done
directly within the DSI phy driver.
Signed-off-by: Raphael Gallais-Pou
---
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 246 ++
1 file changed, 215 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index 82fff9e84345..283f546a7198 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -7,7 +7,9 @@
*/
#include
+#include
#include
+#include
#include
#include
#include
@@ -77,9 +79,12 @@ enum dsi_color {
struct dw_mipi_dsi_stm {
void __iomem *base;
+ struct device *dev;
struct clk *pllref_clk;
struct clk *pclk;
+ struct clk_hw txbyte_clk;
struct dw_mipi_dsi *dsi;
+ struct dw_mipi_dsi_plat_data pdata;
u32 hw_version;
int lane_min_kbps;
int lane_max_kbps;
@@ -196,29 +201,198 @@ static int dsi_pll_get_params(struct dw_mipi_dsi_stm
*dsi,
return 0;
}
-static int dw_mipi_dsi_phy_init(void *priv_data)
+#define clk_to_dw_mipi_dsi_stm(clk) \
+ container_of(clk, struct dw_mipi_dsi_stm, txbyte_clk)
+
+static void dw_mipi_dsi_clk_disable(struct clk_hw *clk)
{
- struct dw_mipi_dsi_stm *dsi = priv_data;
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(clk);
+
+ DRM_DEBUG_DRIVER("\n");
+
+ /* Disable the DSI PLL */
+ dsi_clear(dsi, DSI_WRPCR, WRPCR_PLLEN);
+
+ /* Disable the regulator */
+ dsi_clear(dsi, DSI_WRPCR, WRPCR_REGEN | WRPCR_BGREN);
+}
+
+static int dw_mipi_dsi_clk_enable(struct clk_hw *clk)
+{
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(clk);
u32 val;
int ret;
+ DRM_DEBUG_DRIVER("\n");
+
/* Enable the regulator */
dsi_set(dsi, DSI_WRPCR, WRPCR_REGEN | WRPCR_BGREN);
- ret = readl_poll_timeout(dsi->base + DSI_WISR, val, val & WISR_RRS,
-SLEEP_US, TIMEOUT_US);
+ ret = readl_poll_timeout_atomic(dsi->base + DSI_WISR, val, val &
WISR_RRS,
+ SLEEP_US, TIMEOUT_US);
if (ret)
DRM_DEBUG_DRIVER("!TIMEOUT! waiting REGU, let's continue\n");
/* Enable the DSI PLL & wait for its lock */
dsi_set(dsi, DSI_WRPCR, WRPCR_PLLEN);
- ret = readl_poll_timeout(dsi->base + DSI_WISR, val, val & WISR_PLLLS,
-SLEEP_US, TIMEOUT_US);
+ ret = readl_poll_timeout_atomic(dsi->base + DSI_WISR, val, val &
WISR_PLLLS,
+ SLEEP_US, TIMEOUT_US);
if (ret)
DRM_DEBUG_DRIVER("!TIMEOUT! waiting PLL, let's continue\n");
return 0;
}
+static int dw_mipi_dsi_clk_is_enabled(struct clk_hw *hw)
+{
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw);
+
+ return dsi_read(dsi, DSI_WRPCR) & WRPCR_PLLEN;
+}
+
+static unsigned long dw_mipi_dsi_clk_recalc_rate(struct clk_hw *hw,
+unsigned long parent_rate)
+{
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw);
+ unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz;
+ u32 val;
+
+ DRM_DEBUG_DRIVER("\n");
+
+ pll_in_khz = (unsigned int)(parent_rate / 1000);
+
+ val = dsi_read(dsi, DSI_WRPCR);
+
+ idf = (val & WRPCR_IDF) >> 11;
+ if (!idf)
+ idf = 1;
+ ndiv = (val & WRPCR_NDIV) >> 2;
+ odf = int_pow(2, (val & WRPCR_ODF) >> 16);
+
+ /* Get the adjusted pll out value */
+ pll_out_khz = dsi_pll_get_clkout_khz(pll_in_khz, idf, ndiv, odf);
+
+ return (unsigned long)pll_out_khz * 1000;
+}
+
+static long dw_mipi_dsi_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw);
+ unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz;
+ int ret;
+
+ DRM_DEBUG_DRIVER("\n");
+
+ pll_in_khz = (unsigned int)(*parent_rate / 1000);
+
+ /* Compute best pll parameters */
+ idf = 0;
+ ndiv = 0;
+ odf = 0;
+
+ ret = dsi_pll_get_params(dsi, pll_in_khz, rate / 1000,
+&idf, &ndiv, &odf);
+ if (ret)
+ DRM_WARN("Warning dsi_pll_get_params(): bad params\n");
+
+ /* Get the adjusted pll out value */
+ pll_out_khz = dsi_pll_get_clkout_khz(pll_i
[PATCH v2 4/4] arm: dts: st: fix DSI peripheral clock on stm32mp15 boards
In RCC driver, 'DSI_K' is a kernel clock while 'DSI' has pclk4 as parent
clock, which means that it is an APB peripheral clock. Swap the clocks
in the DSI peripheral clock reference.
Signed-off-by: Raphael Gallais-Pou
---
arch/arm/boot/dts/st/stm32mp157.dtsi | 2 +-
arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts | 2 +-
arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts | 2 +-
arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts | 2 +-
arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/st/stm32mp157.dtsi
b/arch/arm/boot/dts/st/stm32mp157.dtsi
index 6197d878894d..97cd24227cef 100644
--- a/arch/arm/boot/dts/st/stm32mp157.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp157.dtsi
@@ -20,7 +20,7 @@ gpu: gpu@5900 {
dsi: dsi@5a00 {
compatible = "st,stm32-dsi";
reg = <0x5a00 0x800>;
- clocks = <&rcc DSI_K>, <&clk_hse>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&clk_hse>, <&rcc DSI_PX>;
clock-names = "pclk", "ref", "px_clk";
phy-dsi-supply = <®18>;
resets = <&rcc DSI_R>;
diff --git a/arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts
b/arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts
index afcd6285890c..8634699cc65e 100644
--- a/arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts
+++ b/arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts
@@ -30,7 +30,7 @@ &cpu1 {
};
&dsi {
- clocks = <&rcc DSI_K>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
};
&gpioz {
diff --git a/arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts
b/arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts
index 39358d902000..3a897fa7e167 100644
--- a/arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts
+++ b/arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts
@@ -36,7 +36,7 @@ &cryp1 {
&dsi {
phy-dsi-supply = <&scmi_reg18>;
- clocks = <&rcc DSI_K>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
};
&gpioz {
diff --git a/arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts
b/arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts
index 07ea765a4553..29d6465b1fe6 100644
--- a/arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts
+++ b/arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts
@@ -35,7 +35,7 @@ &cryp1 {
};
&dsi {
- clocks = <&rcc DSI_K>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
};
&gpioz {
diff --git a/arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts
b/arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts
index 813086ec2489..5acb78f0a084 100644
--- a/arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts
+++ b/arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts
@@ -37,7 +37,7 @@ &cryp1 {
&dsi {
phy-dsi-supply = <&scmi_reg18>;
- clocks = <&rcc DSI_K>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
};
&gpioz {
--
2.25.1
Re: [PATCH v1 1/1] drm/i915/display: Don't use "proxy" headers
On Wed, 29 Nov 2023, Andy Shevchenko wrote: > The driver uses math.h and not util_macros.h. > > Signed-off-by: Andy Shevchenko Thanks, pushed to drm-intel-next. BR, Jani. > --- > drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c > b/drivers/gpu/drm/i915/display/intel_snps_phy.c > index ce5a73a4cc89..bc61e736f9b3 100644 > --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c > +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c > @@ -3,7 +3,7 @@ > * Copyright © 2019 Intel Corporation > */ > > -#include > +#include > > #include "i915_reg.h" > #include "intel_ddi.h" -- Jani Nikula, Intel
Re: [RFC PATCH 2/6] mm/gmem: add arch-independent abstraction to track address mapping status
On 02.12.23 15:50, Pedro Falcato wrote: On Fri, Dec 1, 2023 at 9:23 AM David Hildenbrand wrote: On 28.11.23 13:50, Weixi Zhu wrote: This patch adds an abstraction layer, struct vm_object, that maintains per-process virtual-to-physical mapping status stored in struct gm_mapping. For example, a virtual page may be mapped to a CPU physical page or to a device physical page. Struct vm_object effectively maintains an arch-independent page table, which is defined as a "logical page table". While arch-dependent page table used by a real MMU is named a "physical page table". The logical page table is useful if Linux core MM is extended to handle a unified virtual address space with external accelerators using customized MMUs. Which raises the question why we are dealing with anonymous memory at all? Why not go for shmem if you are already only special-casing VMAs with a MMAP flag right now? That would maybe avoid having to introduce controversial BSD design concepts into Linux, that feel like going a step backwards in time to me and adding *more* MM complexity. In this patch, struct vm_object utilizes a radix tree (xarray) to track where a virtual page is mapped to. This adds extra memory consumption from xarray, but provides a nice abstraction to isolate mapping status from the machine-dependent layer (PTEs). Besides supporting accelerators with external MMUs, struct vm_object is planned to further union with i_pages in struct address_mapping for file-backed memory. A file already has a tree structure (pagecache) to manage the pages that are theoretically mapped. It's easy to translate from a VMA to a page inside that tree structure that is currently not present in page tables. Why the need for that tree structure if you can just remove anon memory from the picture? The idea of struct vm_object is originated from FreeBSD VM design, which provides a unified abstraction for anonymous memory, file-backed memory, page cache and etc[1]. :/ Currently, Linux utilizes a set of hierarchical page walk functions to abstract page table manipulations of different CPU architecture. The problem happens when a device wants to reuse Linux MM code to manage its page table -- the device page table may not be accessible to the CPU. Existing solution like Linux HMM utilizes the MMU notifier mechanisms to invoke device-specific MMU functions, but relies on encoding the mapping status on the CPU page table entries. This entangles machine-independent code with machine-dependent code, and also brings unnecessary restrictions. Why? we have primitives to walk arch page tables in a non-arch specific fashion and are using them all over the place. We even have various mechanisms to map something into the page tables and get the CPU to fault on it, as if it is inaccessible (PROT_NONE as used for NUMA balancing, fake swap entries). The PTE size and format vary arch by arch, which harms the extensibility. Not really. We might have some features limited to some architectures because of the lack of PTE bits. And usually the problem is that people don't care enough about enabling these features on older architectures. If we ever *really* need more space for sw-defined data, it would be possible to allocate auxiliary data for page tables only where required (where the features apply), instead of crafting a completely new, auxiliary datastructure with it's own locking. So far it was not required to enable the feature we need on the architectures we care about. [1] https://docs.freebsd.org/en/articles/vm-design/ In the cover letter you have: "The future plan of logical page table is to provide a generic abstraction layer that support common anonymous memory (I am looking at you, transparent huge pages) and file-backed memory." Which I doubt will happen; there is little interest in making anonymous memory management slower, more serialized, and wasting more memory on metadata. Also worth noting that: 1) Mach VM (which FreeBSD inherited, from the old BSD) vm_objects aren't quite what's being stated here, rather they are somewhat replacements for both anon_vma and address_space[1]. Very similarly to Linux, they take pages from vm_objects and map them in page tables using pmap (the big difference is anon memory, which has its bookkeeping in page tables, on Linux) 2) These vm_objects were a horrendous mistake (see CoW chaining) and FreeBSD has to go to horrendous lengths to make them tolerable. The UVM paper/dissertation (by Charles Cranor) talks about these issues at length, and 20 years later it's still true. 3) Despite Linux MM having its warts, it's probably correct to consider it a solid improvement over FreeBSD MM or NetBSD UVM And, finally, randomly tacking on core MM concepts from other systems is at best a *really weird* idea. Particularly when they aren't even what was stated! Can you read my mind? :) thanks for noting all that, with which I 100% agree. -- Cheers, David / dhildenb
Re: [PATCH] drm/radeon/trinity_dpm: fix a memleak in trinity_parse_power_table
On Mon, Dec 4, 2023 at 5:55 PM Zhipeng Lu wrote:
>
> The rdev->pm.dpm.ps allocated by kcalloc should be freed in every
> following error-handling path. However, in the error-handling of
> rdev->pm.power_state[i].clock_info the rdev->pm.dpm.ps is not freed,
> resulting in a memleak in this function.
>
> Fixes: d70229f70447 ("drm/radeon/kms: add dpm support for trinity asics")
> Signed-off-by: Zhipeng Lu
> ---
> drivers/gpu/drm/radeon/trinity_dpm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/radeon/trinity_dpm.c
> b/drivers/gpu/drm/radeon/trinity_dpm.c
> index 08ea1c864cb2..8bf56fb7b933 100644
> --- a/drivers/gpu/drm/radeon/trinity_dpm.c
> +++ b/drivers/gpu/drm/radeon/trinity_dpm.c
> @@ -1727,6 +1727,7 @@ static int trinity_parse_power_table(struct
> radeon_device *rdev)
> non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *)
>
> &non_clock_info_array->nonClockInfo[non_clock_array_index];
> if (!rdev->pm.power_state[i].clock_info)
> + kfree(rdev->pm.dpm.ps);
I just confirmed the memleak problem in the code. But the fix has a
problem, it lacks the brackets.
diff --git a/drivers/gpu/drm/radeon/trinity_dpm.c
b/drivers/gpu/drm/radeon/trinity_dpm.c
index 08ea1c864cb2..ef1cc7bad20a 100644
--- a/drivers/gpu/drm/radeon/trinity_dpm.c
+++ b/drivers/gpu/drm/radeon/trinity_dpm.c
@@ -1726,8 +1726,10 @@ static int trinity_parse_power_table(struct
radeon_device *rdev)
non_clock_array_index = power_state->v2.nonClockInfoIndex;
non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *)
&non_clock_info_array->nonClockInfo[non_clock_array_index];
- if (!rdev->pm.power_state[i].clock_info)
+ if (!rdev->pm.power_state[i].clock_info) {
+ kfree(rdev->pm.dpm.ps);
return -EINVAL;
+ }
ps = kzalloc(sizeof(struct sumo_ps), GFP_KERNEL);
if (ps == NULL) {
kfree(rdev->pm.dpm.ps);
> return -EINVAL;
> ps = kzalloc(sizeof(struct sumo_ps), GFP_KERNEL);
> if (ps == NULL) {
> --
> 2.34.1
>
Re: [PATCH v2 6/6] x86/vmware: Add TDX hypercall support
On Fri, Dec 01, 2023 at 03:24:52PM -0800, Alexey Makhalov wrote:
> +#ifdef CONFIG_INTEL_TDX_GUEST
> +/* __tdx_hypercall() is not exported. So, export the wrapper */
> +void vmware_tdx_hypercall_args(struct tdx_module_args *args)
> +{
> + __tdx_hypercall(args);
> +}
> +EXPORT_SYMBOL_GPL(vmware_tdx_hypercall_args);
Uuuh, lovely. I'd like to see what the TDX folks think about this
export first.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 1/6] x86/vmware: Move common macros to vmware.h
On Fri, Dec 01, 2023 at 03:24:47PM -0800, Alexey Makhalov wrote: > Move VMware hypercall macros to vmware.h as a preparation step > for the next commit. No functional changes besides exporting "next commit" in git is ambiguous. Get rid of such formulations. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 2/6] x86/vmware: Introduce vmware_hypercall API
On Fri, Dec 01, 2023 at 03:24:48PM -0800, Alexey Makhalov wrote:
> Introducing vmware_hypercall family of functions as a common
> implementation to be used by the VMware guest code and virtual
> device drivers in architecture independent manner.
>
> By analogy with KVM hypercall API, vmware_hypercallX and
> vmware_hypercall_hb_{out,in} set of functions was added to
> achieve that. Architecture specific implementation should be
> hidden inside.
Pls read section "2) Describe your changes" in
Documentation/process/submitting-patches.rst for more details on how to
formulate your commit messages.
Also, see section "Changelog" in Documentation/process/maintainer-tip.rst
More specifically:
"Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour."
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: Kernel problem with multiseat on one card
Hi Kernel list, I'm the IT person of a school, earlier we used multiseatcomputers for the school, i have maded with a XGL implementation and it works fine but not so fantastic. The school wants that i build new computers but the XGL project is too outdated so i can't use it anymore. How can i make a multiseatcomputer with more then one user on one card with systemd? I have asked already to the makers of systemd but they said it's a kernel problem. With Systemd loginctl and the nouveau drivers you have this: ─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0 │ [MASTER] drm:card0 │ ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-DVI-D-1 │ │ [MASTER] drm:card0-DVI-D-1 │ ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-HDMI-A-1 │ │ [MASTER] drm:card0-HDMI-A-1 │ └─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-VGA-1 │ [MASTER] drm:card0-VGA-1 ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/renderD128 │ drm:renderD128 ├─/sys/devices/pci:00/:00:03.1/:08:00.0/graphics/fb0 │ graphics:fb0 "nouveaudrmfb" So it will be: loginctl attach seat1 /sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-VGA-1 For the seat1 (the VGA d-sub output for seat1 and the other HDMI output for seat0) and of course the mouse and keyboard. When you do this, all the graphics outputs are on the second seat (seat1) and not anymore on the first seat. So i need to move only the VGA output to seat1 and not all the outputs. Do you expect that GUI output is on both seats? I want the result that i have one seat (seat0) on the VGA/D-sub output of the graphic card a second seat (seat1) on the HDMI output of the graphic card and a third seat (seat2) on the DVI output of the graphic card In theory it should work with the following: loginctl attach seat1 /sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-HDMI-A-1 loginctl attach seat2 /sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-DVI-D-1 But when i do this all the outputs will be used on seat2 (because they do all the outputs automaticly to the latest attached seat) When i install the proprietary Nvidia drivers, i have the following: [MASTER] pci::08:00.0 │ ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0 │ │ [MASTER] drm:card0 │ └─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/renderD128 │ drm:renderD128 ─/sys/devices/platform/efi-framebuffer.0/graphics/fb0 │ graphics:fb0 "EFI VGA" So no VGA, DVI or HDMI items. Then report to the GitHub tracker [1]. Thanks. [1]: https://github.com/NVIDIA/open-gpu-kernel-modules/issues
Re: Re: [PATCH] drm/radeon/dpm: fix a memleak in sumo_parse_power_table
> Am 03.12.23 um 18:16 schrieb Zhipeng Lu:
> > The rdev->pm.dpm.ps allocated by kcalloc should be freed in every
> > following error-handling path. However, in the error-handling of
> > rdev->pm.power_state[i].clock_info the rdev->pm.dpm.ps is not freed,
> > resulting in a memleak in this function.
> >
> > Fixes: 80ea2c129c76 ("drm/radeon/kms: add dpm support for sumo asics (v2)")
> > Signed-off-by: Zhipeng Lu
> > ---
> > drivers/gpu/drm/radeon/sumo_dpm.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/radeon/sumo_dpm.c
> > b/drivers/gpu/drm/radeon/sumo_dpm.c
> > index f74f381af05f..bde640053708 100644
> > --- a/drivers/gpu/drm/radeon/sumo_dpm.c
> > +++ b/drivers/gpu/drm/radeon/sumo_dpm.c
> > @@ -1494,6 +1494,7 @@ static int sumo_parse_power_table(struct
> > radeon_device *rdev)
> > non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *)
> >
> > &non_clock_info_array->nonClockInfo[non_clock_array_index];
> > if (!rdev->pm.power_state[i].clock_info)
> > + kfree(rdev->pm.dpm.ps);
> > return -EINVAL;
>
> That change is obviously not correct since you now always return -EINVAL.
>
> You need to at least add {} here.
>
I'm sorry for my mistake and I'll send a new patch soon.
Regards,
Zhipeng
[PATCH] [v2] drm/radeon/dpm: fix a memleak in sumo_parse_power_table
The rdev->pm.dpm.ps allocated by kcalloc should be freed in every
following error-handling path. However, in the error-handling of
rdev->pm.power_state[i].clock_info the rdev->pm.dpm.ps is not freed,
resulting in a memleak in this function.
Fixes: 80ea2c129c76 ("drm/radeon/kms: add dpm support for sumo asics (v2)")
Signed-off-by: Zhipeng Lu
---
Changelog:
v2: Adding {} to make if statement correct.
---
drivers/gpu/drm/radeon/sumo_dpm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/sumo_dpm.c
b/drivers/gpu/drm/radeon/sumo_dpm.c
index f74f381af05f..d49c145db437 100644
--- a/drivers/gpu/drm/radeon/sumo_dpm.c
+++ b/drivers/gpu/drm/radeon/sumo_dpm.c
@@ -1493,8 +1493,10 @@ static int sumo_parse_power_table(struct radeon_device
*rdev)
non_clock_array_index = power_state->v2.nonClockInfoIndex;
non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *)
&non_clock_info_array->nonClockInfo[non_clock_array_index];
- if (!rdev->pm.power_state[i].clock_info)
+ if (!rdev->pm.power_state[i].clock_info) {
+ kfree(rdev->pm.dpm.ps);
return -EINVAL;
+ }
ps = kzalloc(sizeof(struct sumo_ps), GFP_KERNEL);
if (ps == NULL) {
kfree(rdev->pm.dpm.ps);
--
2.34.1
[PATCH] [v2] drm/vmwgfx: fix a memleak in vmw_gmrid_man_get_node
When ida_alloc_max fails, resources allocated before should be freed,
including *res allocated by kmalloc and ttm_resource_init.
Fixes: d3bcb4b02fe9 ("drm/vmwgfx: switch the TTM backends to self alloc")
Signed-off-by: Zhipeng Lu
---
Changelog:
v2: Adding {} to correct the if statement
---
drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
index ceb4d3d3b965..a0b47c9b33f5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c
@@ -64,8 +64,11 @@ static int vmw_gmrid_man_get_node(struct
ttm_resource_manager *man,
ttm_resource_init(bo, place, *res);
id = ida_alloc_max(&gman->gmr_ida, gman->max_gmr_ids - 1, GFP_KERNEL);
- if (id < 0)
+ if (id < 0) {
+ ttm_resource_fini(man, *res);
+ kfree(*res);
return id;
+ }
spin_lock(&gman->lock);
--
2.34.1
[PATCH] [v2] drm/radeon/trinity_dpm: fix a memleak in trinity_parse_power_table
The rdev->pm.dpm.ps allocated by kcalloc should be freed in every
following error-handling path. However, in the error-handling of
rdev->pm.power_state[i].clock_info the rdev->pm.dpm.ps is not freed,
resulting in a memleak in this function.
Fixes: d70229f70447 ("drm/radeon/kms: add dpm support for trinity asics")
Signed-off-by: Zhipeng Lu
---
Changelog:
v2: Adding {} to correct the if statement.
---
drivers/gpu/drm/radeon/trinity_dpm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/trinity_dpm.c
b/drivers/gpu/drm/radeon/trinity_dpm.c
index 08ea1c864cb2..ef1cc7bad20a 100644
--- a/drivers/gpu/drm/radeon/trinity_dpm.c
+++ b/drivers/gpu/drm/radeon/trinity_dpm.c
@@ -1726,8 +1726,10 @@ static int trinity_parse_power_table(struct
radeon_device *rdev)
non_clock_array_index = power_state->v2.nonClockInfoIndex;
non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *)
&non_clock_info_array->nonClockInfo[non_clock_array_index];
- if (!rdev->pm.power_state[i].clock_info)
+ if (!rdev->pm.power_state[i].clock_info) {
+ kfree(rdev->pm.dpm.ps);
return -EINVAL;
+ }
ps = kzalloc(sizeof(struct sumo_ps), GFP_KERNEL);
if (ps == NULL) {
kfree(rdev->pm.dpm.ps);
--
2.34.1
Re: Kernel problem with multiseat on one card
Op 2/12/2023 om 14:18 schreef Bagas Sanjaya: On Thu, Nov 30, 2023 at 11:48:24AM +0100, Gert Vanhaerents wrote: Hi Kernel list, I'm the IT person of a school, earlier we used multiseatcomputers for the school, i have maded with a XGL implementation and it works fine but not so fantastic. The school wants that i build new computers but the XGL project is too outdated so i can't use it anymore. How can i make a multiseatcomputer with more then one user on one card with systemd? I have asked already to the makers of systemd but they said it's a kernel problem. With Systemd loginctl and the nouveau drivers you have this: ─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0 │ [MASTER] drm:card0 │ ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-DVI-D-1 │ │ [MASTER] drm:card0-DVI-D-1 │ ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-HDMI-A-1 │ │ [MASTER] drm:card0-HDMI-A-1 │ └─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-VGA-1 │ [MASTER] drm:card0-VGA-1 ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/renderD128 │ drm:renderD128 ├─/sys/devices/pci:00/:00:03.1/:08:00.0/graphics/fb0 │ graphics:fb0 "nouveaudrmfb" So it will be: loginctl attach seat1 /sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-VGA-1 For the seat1 (the VGA d-sub output for seat1 and the other HDMI output for seat0) and of course the mouse and keyboard. When you do this, all the graphics outputs are on the second seat (seat1) and not anymore on the first seat. So i need to move only the VGA output to seat1 and not all the outputs. Do you expect that GUI output is on both seats? I want the result that i have one seat (seat0) on the VGA/D-sub output of the graphic card a second seat (seat1) on the HDMI output of the graphic card and a third seat (seat2) on the DVI output of the graphic card In theory it should work with the following: loginctl attach seat1 /sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-HDMI-A-1 loginctl attach seat2 /sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0/card0-DVI-D-1 But when i do this all the outputs will be used on seat2 (because they do all the outputs automaticly to the latest attached seat) When i install the proprietary Nvidia drivers, i have the following: [MASTER] pci::08:00.0 │ ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0 │ │ [MASTER] drm:card0 │ └─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/renderD128 │ drm:renderD128 ─/sys/devices/platform/efi-framebuffer.0/graphics/fb0 │ graphics:fb0 "EFI VGA" So no VGA, DVI or HDMI items. Then report to the GitHub tracker [1]. Thanks. [1]: https://github.com/NVIDIA/open-gpu-kernel-modules/issues
Re: [Nouveau] Kernel problem with multiseat on one card
Op 2/12/2023 om 16:28 schreef Timur Tabi: On Sat, 2023-12-02 at 20:18 +0700, Bagas Sanjaya wrote: When i install the proprietary Nvidia drivers, i have the following: [MASTER] pci::08:00.0 │ ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0 │ │ [MASTER] drm:card0 │ └─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/renderD128 │ drm:renderD128 ─/sys/devices/platform/efi-framebuffer.0/graphics/fb0 │ graphics:fb0 "EFI VGA" So no VGA, DVI or HDMI items. Then report to the GitHub tracker [1]. Thanks. [1]:https://github.com/NVIDIA/open-gpu-kernel-modules/issues No, do NOT report this on the Github tracker! That github tracker is ONLY for bugs that occur with OpenRM (the "Open GPU Kernel Module") but not with the the proprietary driver. If you have a bug with the the Nvidia proprietary driver, that must be reported on the Nvidia forum instead: https://forums.developer.nvidia.com/c/gpu-graphics/linux/148 OK i will report it to nvidia. But with the nouveau drivers it's also not working. Are you sure it's not a kernel problem? Because according to systemd it would be a kernel problem. (personaly i am also thinking it's a driver problem)
Re: [Nouveau] Kernel problem with multiseat on one card
Op 4/12/2023 om 9:51 schreef Gert Vanhaerents: Op 2/12/2023 om 16:28 schreef Timur Tabi: On Sat, 2023-12-02 at 20:18 +0700, Bagas Sanjaya wrote: When i install the proprietary Nvidia drivers, i have the following: [MASTER] pci::08:00.0 │ ├─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/card0 │ │ [MASTER] drm:card0 │ └─/sys/devices/pci:00/:00:03.1/:08:00.0/drm/renderD128 │ drm:renderD128 ─/sys/devices/platform/efi-framebuffer.0/graphics/fb0 │ graphics:fb0 "EFI VGA" So no VGA, DVI or HDMI items. Then report to the GitHub tracker [1]. Thanks. [1]: https://github.com/NVIDIA/open-gpu-kernel-modules/issues No, do NOT report this on the Github tracker! That github tracker is ONLY for bugs that occur with OpenRM (the "Open GPU Kernel Module") but not with the the proprietary driver. If you have a bug with the the Nvidia proprietary driver, that must be reported on the Nvidia forum instead: https://forums.developer.nvidia.com/c/gpu-graphics/linux/148 OK i will report it to nvidia. But with the nouveau drivers it's also not working. Are you sure it's not a kernel problem? Because according to systemd it would be a kernel problem. (personaly i am also thinking it's a driver problem)
Re: [PATCH v4 1/4] drm/msm/mdss: switch mdss to use devm_of_icc_get()
On 2.12.2023 23:42, Dmitry Baryshkov wrote: > Stop using hand-written reset function for ICC release, use > devm_of_icc_get() instead. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH v4 3/4] drm/msm/mdss: inline msm_mdss_icc_request_bw()
On 2.12.2023 23:42, Dmitry Baryshkov wrote: > There are just two places where we set the bandwidth: in the resume and > in the suspend paths. Drop the wrapping function > msm_mdss_icc_request_bw() and call icc_set_bw() directly. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH v2] drm/display/dp: Add the remaining Square PHY patterns DPCD register definitions
On Thu, 30 Nov 2023, Khaled Almahallawy wrote: > DP2.1 Specs added new DPCDs definitions for square pattern configs[1] > These new definitions are used for UHBR Source Transmitter > Equalizations tests[2]. Add the 3 new values for square pattern. > > v2: rebase > > [1]: DP2.1 Specs - 2.12.3.6.5 Square Pattern > [2]: DP2.1 PHY CTS specs - 4.3 UHBR Source Transmitter Equalization > > Cc: Jani Nikula > Cc: Imre Deak > Cc: Lee Shawn C > Signed-off-by: Khaled Almahallawy Thanks for the patch, pushed to drm-misc-next. BR, Jani. > --- > include/drm/display/drm_dp.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 83d2039c018b..3731828825bd 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -651,6 +651,9 @@ > # define DP_LINK_QUAL_PATTERN_PRSBS31 0x38 > # define DP_LINK_QUAL_PATTERN_CUSTOM0x40 > # define DP_LINK_QUAL_PATTERN_SQUARE0x48 > +# define DP_LINK_QUAL_PATTERN_SQUARE_PRESHOOT_DISABLED 0x49 > +# define DP_LINK_QUAL_PATTERN_SQUARE_DEEMPHASIS_DISABLED 0x4a > +# define DP_LINK_QUAL_PATTERN_SQUARE_PRESHOOT_DEEMPHASIS_DISABLED0x4b > > #define DP_TRAINING_LANE0_1_SET2 0x10f > #define DP_TRAINING_LANE2_3_SET2 0x110 -- Jani Nikula, Intel
Re: [PATCH v3 3/3] drm/panfrost: Synchronize and disable interrupts before powering off
Il 01/12/23 12:14, Boris Brezillon ha scritto:
On Fri, 1 Dec 2023 11:40:27 +0100
AngeloGioacchino Del Regno
wrote:
To make sure that we don't unintentionally perform any unclocked and/or
unpowered R/W operation on GPU registers, before turning off clocks and
regulators we must make sure that no GPU, JOB or MMU ISR execution is
pending: doing that required to add a mechanism to synchronize the
^ requires the addition of a mechanism...
interrupts on suspend.
Add functions panfrost_{gpu,job,mmu}_suspend_irq() which will perform
interrupts masking and ISR execution synchronization, and then call
those in the panfrost_device_runtime_suspend() handler in the exact
sequence of job (may require mmu!) -> mmu -> gpu.
As a side note, JOB and MMU suspend_irq functions needed some special
treatment: as their interrupt handlers will unmask interrupts, it was
necessary to add a bitmap for `is_suspended` which is used to address
to add an `is_suspended` bitmap which is used...
the possible corner case of unintentional IRQ unmasking because of ISR
execution after a call to synchronize_irq().
Also fixes the case where the interrupt handler is called when the
device is suspended because the IRQ line is shared with another device.
No need to update the commit message for that though.
At resume, clear each is_suspended bit in the reset path of JOB/MMU
to allow unmasking the interrupts.
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/panfrost/panfrost_device.c | 3 +++
drivers/gpu/drm/panfrost/panfrost_device.h | 7 +++
drivers/gpu/drm/panfrost/panfrost_gpu.c| 6 ++
drivers/gpu/drm/panfrost/panfrost_gpu.h| 1 +
drivers/gpu/drm/panfrost/panfrost_job.c| 20 +---
drivers/gpu/drm/panfrost/panfrost_job.h| 1 +
drivers/gpu/drm/panfrost/panfrost_mmu.c| 19 ---
drivers/gpu/drm/panfrost/panfrost_mmu.h| 1 +
8 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
b/drivers/gpu/drm/panfrost/panfrost_device.c
index c90ad5ee34e7..a45e4addcc19 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -421,6 +421,9 @@ static int panfrost_device_runtime_suspend(struct device
*dev)
return -EBUSY;
panfrost_devfreq_suspend(pfdev);
+ panfrost_job_suspend_irq(pfdev);
+ panfrost_mmu_suspend_irq(pfdev);
+ panfrost_gpu_suspend_irq(pfdev);
panfrost_gpu_power_off(pfdev);
return 0;
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index 54a8aad54259..5c24f01f8904 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -25,6 +25,12 @@ struct panfrost_perfcnt;
#define NUM_JOB_SLOTS 3
#define MAX_PM_DOMAINS 5
+enum panfrost_drv_comp_bits {
+ PANFROST_COMP_BIT_MMU,
+ PANFROST_COMP_BIT_JOB,
I think we need one for the GPU interrupt too, for the
irq-line-is-shared-with-another-device thing I was mentioning.
Yes, I've also reordered the entries by name for v4.
+ PANFROST_COMP_BIT_MAX
+};
+
/**
* enum panfrost_gpu_pm - Supported kernel power management features
* @GPU_PM_CLK_DIS: Allow disabling clocks during system suspend
@@ -109,6 +115,7 @@ struct panfrost_device {
struct panfrost_features features;
const struct panfrost_compatible *comp;
+ DECLARE_BITMAP(is_suspended, PANFROST_COMP_BIT_MAX);
spinlock_t as_lock;
unsigned long as_in_use_mask;
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c
b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 7adc4441fa14..3a6a4fe7aca1 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -452,6 +452,12 @@ void panfrost_gpu_power_off(struct panfrost_device *pfdev)
dev_err(pfdev->dev, "l2 power transition timeout");
}
+void panfrost_gpu_suspend_irq(struct panfrost_device *pfdev)
+{
set_bit(PANFROST_COMP_BIT_GPU, pfdev->is_suspended);
here, and an extra check in panfrost_gpu_irq_handler() to bail out
before the register accesses if PANFROST_COMP_BIT_GPU is set.
Right.
+ gpu_write(pfdev, GPU_INT_MASK, 0);
+ synchronize_irq(pfdev->gpu_irq);
+}
+
int panfrost_gpu_init(struct panfrost_device *pfdev)
{
int err;
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.h
b/drivers/gpu/drm/panfrost/panfrost_gpu.h
index 876fdad9f721..d841b86504ea 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.h
@@ -15,6 +15,7 @@ u32 panfrost_gpu_get_latest_flush_id(struct panfrost_device
*pfdev);
int panfrost_gpu_soft_reset(struct panfrost_device *pfdev);
void panfrost_gpu_power_on(struct panfrost_device *pfdev);
void panfrost_gpu_power_off(struct panfrost_device *pfdev);
+void panfrost_gpu_suspend_irq(struct
Re: [PATCH v3 3/3] drm/panfrost: Synchronize and disable interrupts before powering off
Il 01/12/23 13:34, Steven Price ha scritto:
On 01/12/2023 11:14, Boris Brezillon wrote:
On Fri, 1 Dec 2023 11:40:27 +0100
AngeloGioacchino Del Regno
wrote:
To make sure that we don't unintentionally perform any unclocked and/or
unpowered R/W operation on GPU registers, before turning off clocks and
regulators we must make sure that no GPU, JOB or MMU ISR execution is
pending: doing that required to add a mechanism to synchronize the
^ requires the addition of a mechanism...
interrupts on suspend.
Add functions panfrost_{gpu,job,mmu}_suspend_irq() which will perform
interrupts masking and ISR execution synchronization, and then call
those in the panfrost_device_runtime_suspend() handler in the exact
sequence of job (may require mmu!) -> mmu -> gpu.
As a side note, JOB and MMU suspend_irq functions needed some special
treatment: as their interrupt handlers will unmask interrupts, it was
necessary to add a bitmap for `is_suspended` which is used to address
to add an `is_suspended` bitmap which is used...
the possible corner case of unintentional IRQ unmasking because of ISR
execution after a call to synchronize_irq().
Also fixes the case where the interrupt handler is called when the
device is suspended because the IRQ line is shared with another device.
No need to update the commit message for that though.
At resume, clear each is_suspended bit in the reset path of JOB/MMU
to allow unmasking the interrupts.
Signed-off-by: AngeloGioacchino Del Regno
---
static void panfrost_job_handle_err(struct panfrost_device *pfdev,
struct panfrost_job *job,
unsigned int js)
@@ -792,9 +802,13 @@ static irqreturn_t panfrost_job_irq_handler_thread(int
irq, void *data)
struct panfrost_device *pfdev = data;
panfrost_job_handle_irqs(pfdev);
- job_write(pfdev, JOB_INT_MASK,
- GENMASK(16 + NUM_JOB_SLOTS - 1, 16) |
- GENMASK(NUM_JOB_SLOTS - 1, 0));
+
+ /* Enable interrupts only if we're not about to get suspended */
+ if (!test_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended))
+ job_write(pfdev, JOB_INT_MASK,
+ GENMASK(16 + NUM_JOB_SLOTS - 1, 16) |
+ GENMASK(NUM_JOB_SLOTS - 1, 0));
+
Missing if (test_bit(PANFROST_COMP_BIT_JOB, pfdev->is_suspended)) in
panfrost_job_irq_handler(), to make sure you don't access the registers
if the GPU is suspended.
I think generally these IRQ handler functions should simply check the
is_suspended flag and early out if the flag is set. It's not the
re-enabling of the interrupts specifically that we want to gate - it's
any access to the hardware as in the shared-IRQ case the GPU might
already have been powered down/unclocked.
Yes, in the thread handler we're still powered, because we are synchronizing
irqs - adding the test_bit in the hardirq handler will prevent scheduling the
irq_handler_thread.
What the test_bit() here does is to allow us to handle the last interrupt(s)
(synchronize_irqs() in the suspend function) before cutting off power, without
unwillingly re-enabling the job (or mmu in panfrost_mmu.c) interrupts.
Cheers,
Angelo
Re: [PATCH] drm/msm/adreno: Drop WARN_ON from patchid lookup for new GPUs
On 26.10.2023 21:16, Konrad Dybcio wrote:
>
>
> On 10/23/23 22:20, Rob Clark wrote:
>> On Mon, Oct 23, 2023 at 12:56 PM Konrad Dybcio
>> wrote:
>>>
>>>
>>>
>>> On 10/23/23 21:42, Rob Clark wrote:
On Mon, Oct 23, 2023 at 7:29 AM Konrad Dybcio
wrote:
>
> New GPUs still use the lower 2 bytes of the chip id (in whatever form
> it comes) to signify silicon revision. Drop the warning that makes it
> sound as if that was unintended.
>
> Fixes: 90b593ce1c9e ("drm/msm/adreno: Switch to chip-id for identifying
> GPU")
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 80b3f6312116..9a1ec42155fd 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -203,11 +203,6 @@ struct adreno_platform_config {
>
> static inline uint8_t adreno_patchid(const struct adreno_gpu *gpu)
> {
> - /* It is probably ok to assume legacy "adreno_rev" format
> - * for all a6xx devices, but probably best to limit this
> - * to older things.
> - */
> - WARN_ON_ONCE(gpu->info->family >= ADRENO_6XX_GEN1);
Maybe just change it to ADRENO_6XX_GEN4?
>>> That also applies to 700
>>
>> Then the warn is warning about what it is supposed to ;-)
>>
>> I guess this is coming from a6xx_gmu_fw_start()? I think we need a
>> different way to construct the gmu chipid, since the point of this was
>> to not depend on the low 8b having any particular meaning. Perhaps we
>> should just get the gmu chipid from the device table.
> Guess that could work as well..
Well, I realized that we already sorta do this..
MAJ is always set to 7 (duh)
MIN has a lookup table that will expand with future additions
PATCHID needs to vary, and that should be CHIPID & 0xff
Konrad
[PATCH v4 0/3] drm/panfrost: Fix poweroff and sync IRQs for suspend
Changes in v4: - Added checks for is_suspended bits in hardirqs - Added GPU suspended bit (and handling of it) - Reordered panfrost_drv_comp_bits entries - Commit description fixes Changes in v3: - Removed useless GPU_INT_CLEAR write in suspend path - Changed to clear suspend bits in job/mmu reset path This series contains a fast fix for the basic GPU poweroff functionality and goes further by implementing interrupt masking and synchronization before suspend. For more information, please look at the conversation at [1], which explains the regression seen with the poweroff commit and the initial approaches taken to solve that. Cheers! [1]: https://lore.kernel.org/all/20231123095320.41433-1-angelogioacchino.delre...@collabora.com/ AngeloGioacchino Del Regno (3): drm/panfrost: Ignore core_mask for poweroff and disable PWRTRANS irq drm/panfrost: Add gpu_irq, mmu_irq to struct panfrost_device drm/panfrost: Synchronize and disable interrupts before powering off drivers/gpu/drm/panfrost/panfrost_device.c | 3 ++ drivers/gpu/drm/panfrost/panfrost_device.h | 10 ++ drivers/gpu/drm/panfrost/panfrost_gpu.c| 40 -- drivers/gpu/drm/panfrost/panfrost_gpu.h| 1 + drivers/gpu/drm/panfrost/panfrost_job.c| 26 +++--- drivers/gpu/drm/panfrost/panfrost_job.h| 1 + drivers/gpu/drm/panfrost/panfrost_mmu.c| 32 - drivers/gpu/drm/panfrost/panfrost_mmu.h| 1 + 8 files changed, 91 insertions(+), 23 deletions(-) -- 2.43.0
[PATCH v4 1/3] drm/panfrost: Ignore core_mask for poweroff and disable PWRTRANS irq
Some SoCs may be equipped with a GPU containing two core groups
and this is exactly the case of Samsung's Exynos 5422 featuring
an ARM Mali-T628 MP6 GPU: the support for this GPU in Panfrost
is partial, as this driver currently supports using only one
core group and that's reflected on all parts of it, including
the power on (and power off, previously to this patch) function.
The issue with this is that even though executing the soft reset
operation should power off all cores unconditionally, on at least
one platform we're seeing a crash that seems to be happening due
to an interrupt firing which may be because we are calling power
transition only on the first core group, leaving the second one
unchanged, or because ISR execution was pending before entering
the panfrost_gpu_power_off() function and executed after powering
off the GPU cores, or all of the above.
Finally, solve this by:
- Avoid to enable the power transition interrupt on reset; and
- Ignoring the core_mask and ask the GPU to poweroff both core groups
Fixes: 22aa1a209018 ("drm/panfrost: Really power off GPU cores in
panfrost_gpu_power_off()")
Reviewed-by: Boris Brezillon
Reviewed-by: Steven Price
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/panfrost/panfrost_gpu.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c
b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 09f5e1563ebd..bd41617c5e4b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -78,7 +78,12 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev)
}
gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_MASK_ALL);
- gpu_write(pfdev, GPU_INT_MASK, GPU_IRQ_MASK_ALL);
+
+ /* Only enable the interrupts we care about */
+ gpu_write(pfdev, GPU_INT_MASK,
+ GPU_IRQ_MASK_ERROR |
+ GPU_IRQ_PERFCNT_SAMPLE_COMPLETED |
+ GPU_IRQ_CLEAN_CACHES_COMPLETED);
/*
* All in-flight jobs should have released their cycle
@@ -425,11 +430,10 @@ void panfrost_gpu_power_on(struct panfrost_device *pfdev)
void panfrost_gpu_power_off(struct panfrost_device *pfdev)
{
- u64 core_mask = panfrost_get_core_mask(pfdev);
int ret;
u32 val;
- gpu_write(pfdev, SHADER_PWROFF_LO, pfdev->features.shader_present &
core_mask);
+ gpu_write(pfdev, SHADER_PWROFF_LO, pfdev->features.shader_present);
ret = readl_relaxed_poll_timeout(pfdev->iomem + SHADER_PWRTRANS_LO,
val, !val, 1, 1000);
if (ret)
@@ -441,7 +445,7 @@ void panfrost_gpu_power_off(struct panfrost_device *pfdev)
if (ret)
dev_err(pfdev->dev, "tiler power transition timeout");
- gpu_write(pfdev, L2_PWROFF_LO, pfdev->features.l2_present & core_mask);
+ gpu_write(pfdev, L2_PWROFF_LO, pfdev->features.l2_present);
ret = readl_poll_timeout(pfdev->iomem + L2_PWRTRANS_LO,
val, !val, 0, 1000);
if (ret)
--
2.43.0
[PATCH v4 2/3] drm/panfrost: Add gpu_irq, mmu_irq to struct panfrost_device
In preparation for adding a IRQ synchronization mechanism for PM suspend,
add gpu_irq and mmu_irq variables to struct panfrost_device and change
functions panfrost_gpu_init() and panfrost_mmu_init() to use those.
Reviewed-by: Boris Brezillon
Reviewed-by: Steven Price
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/panfrost/panfrost_device.h | 2 ++
drivers/gpu/drm/panfrost/panfrost_gpu.c| 10 +-
drivers/gpu/drm/panfrost/panfrost_mmu.c| 10 +-
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index 0fc558db6bfd..54a8aad54259 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -94,6 +94,8 @@ struct panfrost_device {
struct device *dev;
struct drm_device *ddev;
struct platform_device *pdev;
+ int gpu_irq;
+ int mmu_irq;
void __iomem *iomem;
struct clk *clock;
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c
b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index bd41617c5e4b..7adc4441fa14 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -454,7 +454,7 @@ void panfrost_gpu_power_off(struct panfrost_device *pfdev)
int panfrost_gpu_init(struct panfrost_device *pfdev)
{
- int err, irq;
+ int err;
err = panfrost_gpu_soft_reset(pfdev);
if (err)
@@ -469,11 +469,11 @@ int panfrost_gpu_init(struct panfrost_device *pfdev)
dma_set_max_seg_size(pfdev->dev, UINT_MAX);
- irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "gpu");
- if (irq < 0)
- return irq;
+ pfdev->gpu_irq =
platform_get_irq_byname(to_platform_device(pfdev->dev), "gpu");
+ if (pfdev->gpu_irq < 0)
+ return pfdev->gpu_irq;
- err = devm_request_irq(pfdev->dev, irq, panfrost_gpu_irq_handler,
+ err = devm_request_irq(pfdev->dev, pfdev->gpu_irq,
panfrost_gpu_irq_handler,
IRQF_SHARED, KBUILD_MODNAME "-gpu", pfdev);
if (err) {
dev_err(pfdev->dev, "failed to request gpu irq");
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index 846dd697c410..ac4296c1e54b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -753,13 +753,13 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int
irq, void *data)
int panfrost_mmu_init(struct panfrost_device *pfdev)
{
- int err, irq;
+ int err;
- irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "mmu");
- if (irq < 0)
- return irq;
+ pfdev->mmu_irq =
platform_get_irq_byname(to_platform_device(pfdev->dev), "mmu");
+ if (pfdev->mmu_irq < 0)
+ return pfdev->mmu_irq;
- err = devm_request_threaded_irq(pfdev->dev, irq,
+ err = devm_request_threaded_irq(pfdev->dev, pfdev->mmu_irq,
panfrost_mmu_irq_handler,
panfrost_mmu_irq_handler_thread,
IRQF_SHARED, KBUILD_MODNAME "-mmu",
--
2.43.0
[PATCH v4 3/3] drm/panfrost: Synchronize and disable interrupts before powering off
To make sure that we don't unintentionally perform any unclocked and/or
unpowered R/W operation on GPU registers, before turning off clocks and
regulators we must make sure that no GPU, JOB or MMU ISR execution is
pending: doing that requires to add a mechanism to synchronize the
interrupts on suspend.
Add functions panfrost_{gpu,job,mmu}_suspend_irq() which will perform
interrupts masking and ISR execution synchronization, and then call
those in the panfrost_device_runtime_suspend() handler in the exact
sequence of job (may require mmu!) -> mmu -> gpu.
As a side note, JOB and MMU suspend_irq functions needed some special
treatment: as their interrupt handlers will unmask interrupts, it was
necessary to add an `is_suspended` bitmap which is used to address the
possible corner case of unintentional IRQ unmasking because of ISR
execution after a call to synchronize_irq().
At resume, clear each is_suspended bit in the reset path of JOB/MMU
to allow unmasking the interrupts.
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/panfrost/panfrost_device.c | 3 +++
drivers/gpu/drm/panfrost/panfrost_device.h | 8 +++
drivers/gpu/drm/panfrost/panfrost_gpu.c| 18 +--
drivers/gpu/drm/panfrost/panfrost_gpu.h| 1 +
drivers/gpu/drm/panfrost/panfrost_job.c| 26 ++
drivers/gpu/drm/panfrost/panfrost_job.h| 1 +
drivers/gpu/drm/panfrost/panfrost_mmu.c| 22 +++---
drivers/gpu/drm/panfrost/panfrost_mmu.h| 1 +
8 files changed, 71 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
b/drivers/gpu/drm/panfrost/panfrost_device.c
index c90ad5ee34e7..a45e4addcc19 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -421,6 +421,9 @@ static int panfrost_device_runtime_suspend(struct device
*dev)
return -EBUSY;
panfrost_devfreq_suspend(pfdev);
+ panfrost_job_suspend_irq(pfdev);
+ panfrost_mmu_suspend_irq(pfdev);
+ panfrost_gpu_suspend_irq(pfdev);
panfrost_gpu_power_off(pfdev);
return 0;
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index 54a8aad54259..62f7e3527385 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -25,6 +25,13 @@ struct panfrost_perfcnt;
#define NUM_JOB_SLOTS 3
#define MAX_PM_DOMAINS 5
+enum panfrost_drv_comp_bits {
+ PANFROST_COMP_BIT_GPU,
+ PANFROST_COMP_BIT_JOB,
+ PANFROST_COMP_BIT_MMU,
+ PANFROST_COMP_BIT_MAX
+};
+
/**
* enum panfrost_gpu_pm - Supported kernel power management features
* @GPU_PM_CLK_DIS: Allow disabling clocks during system suspend
@@ -109,6 +116,7 @@ struct panfrost_device {
struct panfrost_features features;
const struct panfrost_compatible *comp;
+ DECLARE_BITMAP(is_suspended, PANFROST_COMP_BIT_MAX);
spinlock_t as_lock;
unsigned long as_in_use_mask;
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c
b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 7adc4441fa14..9063ce254642 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -22,9 +22,13 @@
static irqreturn_t panfrost_gpu_irq_handler(int irq, void *data)
{
struct panfrost_device *pfdev = data;
- u32 state = gpu_read(pfdev, GPU_INT_STAT);
- u32 fault_status = gpu_read(pfdev, GPU_FAULT_STATUS);
+ u32 fault_status, state;
+ if (test_bit(PANFROST_COMP_BIT_GPU, pfdev->is_suspended))
+ return IRQ_NONE;
+
+ fault_status = gpu_read(pfdev, GPU_FAULT_STATUS);
+ state = gpu_read(pfdev, GPU_INT_STAT);
if (!state)
return IRQ_NONE;
@@ -61,6 +65,8 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev)
gpu_write(pfdev, GPU_INT_MASK, 0);
gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_RESET_COMPLETED);
+ clear_bit(PANFROST_COMP_BIT_GPU, pfdev->is_suspended);
+
gpu_write(pfdev, GPU_CMD, GPU_CMD_SOFT_RESET);
ret = readl_relaxed_poll_timeout(pfdev->iomem + GPU_INT_RAWSTAT,
val, val & GPU_IRQ_RESET_COMPLETED, 10, 1);
@@ -452,6 +458,14 @@ void panfrost_gpu_power_off(struct panfrost_device *pfdev)
dev_err(pfdev->dev, "l2 power transition timeout");
}
+void panfrost_gpu_suspend_irq(struct panfrost_device *pfdev)
+{
+ set_bit(PANFROST_COMP_BIT_GPU, pfdev->is_suspended);
+
+ gpu_write(pfdev, GPU_INT_MASK, 0);
+ synchronize_irq(pfdev->gpu_irq);
+}
+
int panfrost_gpu_init(struct panfrost_device *pfdev)
{
int err;
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.h
b/drivers/gpu/drm/panfrost/panfrost_gpu.h
index 876fdad9f721..d841b86504ea 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.h
@@ -15,6 +15,7 @@ u32 panfrost_gpu_get_latest_flush_id(struct panfr
[PATCH 1/5] drm/atomic: Move the drm_atomic_state field doc inline
Some fields of drm_atomic_state have been documented in-line, but some
were documented in the main kerneldoc block before the structure.
Since the former is the preferred option in DRM, let's move all the
fields to an inline documentation.
Signed-off-by: Maxime Ripard
---
include/drm/drm_atomic.h | 50
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index cf8e1220a4ac..2a08030fcd75 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -347,24 +347,22 @@ struct __drm_private_objs_state {
/**
* struct drm_atomic_state - the global state object for atomic updates
- * @ref: count of all references to this state (will not be freed until zero)
- * @dev: parent DRM device
- * @async_update: hint for asynchronous plane update
- * @planes: pointer to array of structures with per-plane data
- * @crtcs: pointer to array of CRTC pointers
- * @num_connector: size of the @connectors and @connector_states arrays
- * @connectors: pointer to array of structures with per-connector data
- * @num_private_objs: size of the @private_objs array
- * @private_objs: pointer to array of private object pointers
- * @acquire_ctx: acquire context for this atomic modeset state update
*
* States are added to an atomic update by calling drm_atomic_get_crtc_state(),
* drm_atomic_get_plane_state(), drm_atomic_get_connector_state(), or for
* private state structures, drm_atomic_get_private_obj_state().
*/
struct drm_atomic_state {
+ /**
+* @ref:
+*
+* Count of all references to this update (will not be freed until
zero).
+*/
struct kref ref;
+ /**
+* @dev: Parent DRM Device.
+*/
struct drm_device *dev;
/**
@@ -388,7 +386,12 @@ struct drm_atomic_state {
* flag are not allowed.
*/
bool legacy_cursor_update : 1;
+
+ /**
+* @async_update: hint for asynchronous plane update
+*/
bool async_update : 1;
+
/**
* @duplicated:
*
@@ -398,13 +401,40 @@ struct drm_atomic_state {
* states.
*/
bool duplicated : 1;
+
+ /**
+* @planes: pointer to array of structures with per-plane data
+*/
struct __drm_planes_state *planes;
+
+ /**
+* @crtcs: pointer to array of CRTC pointers
+*/
struct __drm_crtcs_state *crtcs;
+
+ /**
+* @num_connector: size of the @connectors and @connector_states arrays
+*/
int num_connector;
+
+ /**
+* @connectors: pointer to array of structures with per-connector data
+*/
struct __drm_connnectors_state *connectors;
+
+ /**
+* @num_private_objs: size of the @private_objs array
+*/
int num_private_objs;
+
+ /**
+* @private_objs: pointer to array of private object pointers
+*/
struct __drm_private_objs_state *private_objs;
+ /**
+* @acquire_ctx: acquire context for this atomic modeset state update
+*/
struct drm_modeset_acquire_ctx *acquire_ctx;
/**
--
2.43.0
[PATCH 4/5] drm/atomic: Make the drm_atomic_state documentation less ambiguous
The current documentation of drm_atomic_state says that it's the "global
state object". This is confusing since, while it does contain all the
objects affected by an update and their respective states, if an object
isn't affected by this update it won't be part of it.
Thus, it's not truly a "global state", unlike object state structures
that do contain the entire state of a given object.
Signed-off-by: Maxime Ripard
---
include/drm/drm_atomic.h | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 914574b58ae7..81ad7369b90d 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -346,11 +346,19 @@ struct __drm_private_objs_state {
};
/**
- * struct drm_atomic_state - the global state object for atomic updates
+ * struct drm_atomic_state - Atomic Update structure
+ *
+ * This structure is the kernel counterpart of @drm_mode_atomic and contains
+ * all the objects affected by an atomic modeset update and their states.
*
* States are added to an atomic update by calling drm_atomic_get_crtc_state(),
* drm_atomic_get_plane_state(), drm_atomic_get_connector_state(), or for
* private state structures, drm_atomic_get_private_obj_state().
+ *
+ * NOTE: While this structure looks to be global and affecting the whole DRM
+ * device, it only contains the objects affected by the atomic commit.
+ * Unaffected objects will not be part of that update, unless they have been
+ * explicitly added by either the framework or the driver.
*/
struct drm_atomic_state {
/**
--
2.43.0
[PATCH 2/5] drm/atomic: Remove inexistent reference
The num_connectors field documentation mentions a connector_states field
that has never been part of this structure.
Signed-off-by: Maxime Ripard
---
include/drm/drm_atomic.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 2a08030fcd75..13cecdc1257d 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -413,7 +413,7 @@ struct drm_atomic_state {
struct __drm_crtcs_state *crtcs;
/**
-* @num_connector: size of the @connectors and @connector_states arrays
+* @num_connector: size of the @connectors array
*/
int num_connector;
--
2.43.0
[PATCH 5/5] drm/todo: Add entry to rename drm_atomic_state
The name of the structure drm_atomic_state is confusing. Let's add an entry to our todo list to rename it. Signed-off-by: Maxime Ripard --- Documentation/gpu/todo.rst | 18 ++ 1 file changed, 18 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index b62c7fa0c2bc..fe95aea89d67 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -120,6 +120,24 @@ Contact: Daniel Vetter, respective driver maintainers Level: Advanced +Rename drm_atomic_state +--- + +The KMS framework uses two slightly different definitions for the ``state`` +concept. For a given object (plane, CRTC, encoder, etc., so +``drm_$OBJECT_state``), the state is the entire state of that object. However, +at the device level, ``drm_atomic_state`` refers to a state update for a +limited number of objects. + +The state isn't the entire device state anymore, but only the full state of +some objects in that device. This is confusing to newcomers, and +``drm_atomic_state`` should be renamed to something clearer like +``drm_atomic_update``. + +Contact: Maxime Ripard + +Level: Advanced + Fallout from atomic KMS --- -- 2.43.0
[PATCH 3/5] drm/atomic: Rework the object doc a bit
The doc for the planes, crtcs, connectors and private_objs fields
mention that they are pointers to an array of structures with
per-$OBJECT data.
While these fields are indeed pointers to an array, each item of that
array contain a pointer to the object structure affected by the update,
and its old and new state. There's no per-object data there.
Let's rephrase those fields a bit to better match the current situation.
Signed-off-by: Maxime Ripard
---
include/drm/drm_atomic.h | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 13cecdc1257d..914574b58ae7 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -403,12 +403,18 @@ struct drm_atomic_state {
bool duplicated : 1;
/**
-* @planes: pointer to array of structures with per-plane data
+* @planes:
+*
+* Pointer to array of @drm_plane and @drm_plane_state part of this
+* update.
*/
struct __drm_planes_state *planes;
/**
-* @crtcs: pointer to array of CRTC pointers
+* @crtcs:
+*
+* Pointer to array of @drm_crtc and @drm_crtc_state part of this
+* update.
*/
struct __drm_crtcs_state *crtcs;
@@ -418,7 +424,10 @@ struct drm_atomic_state {
int num_connector;
/**
-* @connectors: pointer to array of structures with per-connector data
+* @connectors:
+*
+* Pointer to array of @drm_connector and @drm_connector_state part of
+* this update.
*/
struct __drm_connnectors_state *connectors;
@@ -428,7 +437,10 @@ struct drm_atomic_state {
int num_private_objs;
/**
-* @private_objs: pointer to array of private object pointers
+* @private_objs:
+*
+* Pointer to array of @drm_private_obj and @drm_private_obj_state part
+* of this update.
*/
struct __drm_private_objs_state *private_objs;
--
2.43.0
Re: [PATCH] drm/doc: Define KMS atomic state set
On Fri, Dec 01, 2023 at 08:09:22PM +0200, Ville Syrjälä wrote: > > When I was working on Weston atomic KMS support many years ago, I > > created a framework that emitted KMS property changes only when they > > actually needed changing. By review feedback (*), all that machinery was > > dropped in a re-design, and today Weston always emits all KMS > > properties it knows to program for a specific CRTC update including all > > relevant planes and connectors. > > > > (*) Why do we need to repeat the same state tracking that the kernel > > does anyway, and also risk getting out of sync with the kernel due to > > bugs which then become more difficult to diagnose. I guess (assumed) > > kernel internals leaked to userspace. Oops. > > The kernel does track the full state sure, but it doesn't generally > go out of its way to figure out what specifically changed in that state. > Doing so would be a lot of extra checks, and kinda less convenient to > do inside the driver since at that point the state is already spread > all over the various structures. And the fact that those structures > are a mismash of uapi and internal bits of state (and other metadata > for the single commit that really shouldn't be stored there) doesn't > help matters. I did propose to split the state cleanly into pure uapi > vs. internal stuff but that didn't gain any traction unfortunately. Also, that's not how drivers are modelled, so even though we could possibly figure out the entire state of the device, we don't have any code for it because no one really needs to. Maxime signature.asc Description: PGP signature
[PATCH next] drm/v3d: Fix missing error code in v3d_submit_cpu_ioctl()
Smatch warns:
drivers/gpu/drm/v3d/v3d_submit.c:1222 v3d_submit_cpu_ioctl()
warn: missing error code 'ret'
When there is no job type or job is submitted with wrong number of BOs
it is an error path, ret is zero at this point which is incorrect
return.
Fix this by changing it to -EINVAL.
Fixes: aafc1a2bea67 ("drm/v3d: Add a CPU job submission")
Signed-off-by: Harshit Mogalapalli
---
This is based on static analysis and only compile tested.
---
drivers/gpu/drm/v3d/v3d_submit.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index d7a9da2484fd..fcff41dd2315 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -1219,11 +1219,13 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
/* Every CPU job must have a CPU job user extension */
if (!cpu_job->job_type) {
DRM_DEBUG("CPU job must have a CPU job user extension.\n");
+ ret = -EINVAL;
goto fail;
}
if (args->bo_handle_count !=
cpu_job_bo_handle_count[cpu_job->job_type]) {
DRM_DEBUG("This CPU job was not submitted with the proper
number of BOs.\n");
+ ret = -EINVAL;
goto fail;
}
--
2.39.3
Re: [PATCH RESEND 0/6] drm: simplify support for transparent DRM bridges
On Sun, Dec 03, 2023 at 02:43:27PM +0300, Dmitry Baryshkov wrote: > Greg, could you please ack the last patch to be merged through the > drm-misc tree? You have acked patch 3, but since that time I've added > patches 4-6. That is up to the typec maintainer to ack, not me! thanks, greg k-h
[PATCH] accel/ivpu/37xx: Fix interrupt_clear_with_0 WA initialization
From: Andrzej Kacprowski
Using PCI Device ID/Revision to initialize the interrupt_clear_with_0
workaround is problematic - there are many pre-production
steppings with different behavior, even with the same PCI ID/Revision
Instead of checking for PCI Device ID/Revision, check the VPU
buttress interrupt status register behavior - if this register
is not zero after writing 1s it means there register is RW
instead of RW1C and we need to enable the interrupt_clear_with_0
workaround.
Fixes: 7f34e01f77f8 ("accel/ivpu: Clear specific interrupt status bits on C0")
Signed-off-by: Andrzej Kacprowski
Signed-off-by: Jacek Lawrynowicz
---
drivers/accel/ivpu/ivpu_hw_37xx.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_hw_37xx.c
b/drivers/accel/ivpu/ivpu_hw_37xx.c
index 4ccf1994b97a..d530384f8d60 100644
--- a/drivers/accel/ivpu/ivpu_hw_37xx.c
+++ b/drivers/accel/ivpu/ivpu_hw_37xx.c
@@ -53,10 +53,12 @@
#define ICB_0_1_IRQ_MASK u64)ICB_1_IRQ_MASK) << 32) | ICB_0_IRQ_MASK)
-#define BUTTRESS_IRQ_MASK ((REG_FLD(VPU_37XX_BUTTRESS_INTERRUPT_STAT,
FREQ_CHANGE)) | \
- (REG_FLD(VPU_37XX_BUTTRESS_INTERRUPT_STAT, ATS_ERR))
| \
+#define BUTTRESS_IRQ_MASK ((REG_FLD(VPU_37XX_BUTTRESS_INTERRUPT_STAT,
ATS_ERR)) | \
(REG_FLD(VPU_37XX_BUTTRESS_INTERRUPT_STAT, UFI_ERR)))
+#define BUTTRESS_ALL_IRQ_MASK (BUTTRESS_IRQ_MASK | \
+ (REG_FLD(VPU_37XX_BUTTRESS_INTERRUPT_STAT,
FREQ_CHANGE)))
+
#define BUTTRESS_IRQ_ENABLE_MASK ((u32)~BUTTRESS_IRQ_MASK)
#define BUTTRESS_IRQ_DISABLE_MASK ((u32)-1)
@@ -74,8 +76,12 @@ static void ivpu_hw_wa_init(struct ivpu_device *vdev)
vdev->wa.clear_runtime_mem = false;
vdev->wa.d3hot_after_power_off = true;
- if (ivpu_device_id(vdev) == PCI_DEVICE_ID_MTL && ivpu_revision(vdev) <
4)
+ REGB_WR32(VPU_37XX_BUTTRESS_INTERRUPT_STAT, BUTTRESS_ALL_IRQ_MASK);
+ if (REGB_RD32(VPU_37XX_BUTTRESS_INTERRUPT_STAT) ==
BUTTRESS_ALL_IRQ_MASK) {
+ /* Writing 1s does not clear the interrupt status register */
vdev->wa.interrupt_clear_with_0 = true;
+ REGB_WR32(VPU_37XX_BUTTRESS_INTERRUPT_STAT, 0x0);
+ }
IVPU_PRINT_WA(punit_disabled);
IVPU_PRINT_WA(clear_runtime_mem);
--
2.43.0
Re: [PATCH RESEND 0/6] drm: simplify support for transparent DRM bridges
On Mon, 4 Dec 2023 at 14:21, Greg Kroah-Hartman wrote: > > On Sun, Dec 03, 2023 at 02:43:27PM +0300, Dmitry Baryshkov wrote: > > Greg, could you please ack the last patch to be merged through the > > drm-misc tree? You have acked patch 3, but since that time I've added > > patches 4-6. > > That is up to the typec maintainer to ack, not me! Hmm, true. I didn't notice supporter vs maintainer. Heikki, then we should bug you about that patch. -- With best wishes Dmitry
[PATCH] drm/bridge: nxp-ptn3460: fix i2c_master_send() error checking
The i2c_master_send/recv() functions return negative error codes or the
number of bytes that were able to be sent/received. This code has
two problems. 1) Instead of checking if all the bytes were sent or
received, it checks that at least one byte was sent or received.
2) If there was a partial send/receive then we should return a negative
error code but this code returns success.
Fixes: a9fe713d7d45 ("drm/bridge: Add PTN3460 bridge driver")
Cc: sta...@vger.kernel.org
Signed-off-by: Dan Carpenter
---
This is from static analysis and code review. It's always a concern
when you add stricter error handling that something will break.
drivers/gpu/drm/bridge/nxp-ptn3460.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c
b/drivers/gpu/drm/bridge/nxp-ptn3460.c
index d81920227a8a..9b7eb8c669c1 100644
--- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
+++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
@@ -56,13 +56,13 @@ static int ptn3460_read_bytes(struct ptn3460_bridge
*ptn_bridge, char addr,
ret = i2c_master_send(ptn_bridge->client, &addr, 1);
if (ret <= 0) {
DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
- return ret;
+ return ret ?: -EIO;
}
ret = i2c_master_recv(ptn_bridge->client, buf, len);
- if (ret <= 0) {
+ if (ret != len) {
DRM_ERROR("Failed to recv i2c data, ret=%d\n", ret);
- return ret;
+ return ret < 0 ? ret : -EIO;
}
return 0;
@@ -78,9 +78,9 @@ static int ptn3460_write_byte(struct ptn3460_bridge
*ptn_bridge, char addr,
buf[1] = val;
ret = i2c_master_send(ptn_bridge->client, buf, ARRAY_SIZE(buf));
- if (ret <= 0) {
+ if (ret != ARRAY_SIZE(buf)) {
DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
- return ret;
+ return ret < 0 ? ret : -EIO;
}
return 0;
--
2.42.0
[v3 2/6] riscv: dts: starfive: jh7110: display subsystem
Add the dc controller and hdmi node for the Starfive JH7110 SoC.
Signed-off-by: Keith Zhao
---
.../jh7110-starfive-visionfive-2.dtsi | 134 ++
arch/riscv/boot/dts/starfive/jh7110.dtsi | 49 +++
2 files changed, 183 insertions(+)
diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index b89e9791efa7..6e387e0138c0 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -35,6 +35,25 @@ memory@4000 {
reg = <0x0 0x4000 0x1 0x0>;
};
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ /* vout applies for space from this CMA
+* Without this CMA reservation,
+* vout may not work properly.
+*/
+ linux,cma {
+ compatible = "shared-dma-pool";
+ reusable;
+ size = <0x0 0x2000>;
+ alignment = <0x0 0x1000>;
+ alloc-ranges = <0x0 0x7000 0x0 0x2000>;
+ linux,cma-default;
+ };
+ };
+
gpio-restart {
compatible = "gpio-restart";
gpios = <&sysgpio 35 GPIO_ACTIVE_HIGH>;
@@ -69,6 +88,68 @@ codec {
};
};
+&dc8200 {
+ status = "okay";
+
+ crtc_out: ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dc_out0: port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dc_out_dpi0: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&hdmi_enc>;
+ };
+
+ };
+
+ dc_out1: port@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dc_out_dpi1: endpoint@1 {
+ reg = <1>;
+ remote-endpoint = <&dsi_enc>;
+ };
+
+ };
+ };
+};
+
+&display {
+ status = "okay";
+ ports = <&crtc_out>;
+};
+
+&dsi_encoder {
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ /* input */
+ enc_in: port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ dsi_enc:endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&dc_out_dpi1>;
+ };
+ };
+ /* output */
+ enc_out: port@1 {
+ reg = <1>;
+ /*need add a remote-endpoint to dsi bridge*/
+ };
+ };
+};
+
&dvp_clk {
clock-frequency = <7425>;
};
@@ -89,6 +170,21 @@ &gmac1_rmii_refin {
clock-frequency = <5000>;
};
+&hdmi {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmi_pins>;
+
+ hdmi_in: port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ hdmi_enc: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&dc_out_dpi0>;
+ };
+ };
+};
+
&hdmitx0_pixelclk {
clock-frequency = <29700>;
};
@@ -336,6 +432,40 @@ spi_dev0: spi@0 {
};
&sysgpio {
+ hdmi_pins: hdmi-0 {
+ hdmi-cec-pins {
+ pinmux = ;
+ input-enable;
+ bias-pull-up;
+ };
+
+ hdmi-hpd-pins {
+ pinmux = ;
+ input-enable;
+ bias-disable; /* external pull-up */
+ };
+
+ hdmi-scl-pins {
+ pinmux = ;
+ input-enable;
+ bias-pull-up;
+ };
+
+ hdmi-sda-pins {
+ pinmux = ;
+ input-enable;
+ bias-pull-up;
+ };
+ };
+
i2c0_pins: i2c0-0 {
i2c-pins {
pinmux = ;
};
+
+&voutcrg {
+ status = "okay";
+};
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 45213cdf50dc..df51b9407328 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -344,6 +344,24 @@ tdm_ext: tdm-ext-clock {
#clock-cells = <0>;
};
[v3 1/6] dt-bindings: display: Add yamls for JH7110 display system
StarFive SoCs JH7110 display system:
dc controller, hdmi controller,
encoder, vout syscon.
add the path of yaml file in MAINTAINERS
Signed-off-by: Keith Zhao
---
.../starfive/starfive,display-subsystem.yaml | 104
.../starfive/starfive,dsi-encoder.yaml| 92 ++
.../starfive/starfive,jh7110-dc8200.yaml | 113 ++
.../starfive/starfive,jh7110-inno-hdmi.yaml | 82 +
.../soc/starfive/starfive,jh7110-syscon.yaml | 1 +
MAINTAINERS | 7 ++
6 files changed, 399 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml
create mode 100644
Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml
create mode 100644
Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml
create mode 100644
Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml
diff --git
a/Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml
b/Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml
new file mode 100644
index ..d5ebdba3fb36
--- /dev/null
+++
b/Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id:
http://devicetree.org/schemas/display/starfive/starfive,display-subsystem.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Starfive JH7110 Soc Display SubSystem
+
+maintainers:
+ - Keith Zhao
+ - ShengYang Chen
+
+description:
+ This is the bindings documentation for the JH7110 Soc Display Subsystem that
+ includes front-end video data capture, display controller and display
+ interface. such as HDMI and MIPI.
+
+ JH7110 display pipeline have several components as below description,
+ multi display controllers and corresponding physical interfaces.
+ For different display scenarios, pipe0 and pipe1 maybe binding to different
+ encoder. for example,
+
+ pipe0 binding to HDMI for primary display,
+ pipe1 binding to DSI for external display.
+
+ +--+
+ | |
+ | |
+ ++ | +---+ | +---+ +--+ +--+
+ |+->+ dc controller 0 +--->->+HDMICtl| ->+ PHY +-->+PANEL0+
+ |AXI | | +---+ | +---+ +--+ +--+
+ || | |
+ || | |
+ || | |
+ || | |
+ |APB | | +---+ +-++--+ +---+
+ |+->+ dc controller 1 +--->>+ dsiTx +--->+DPHY +->+ PANEL1+
+ || | +---+ +-++--+ +---+
+ ++ | |
+ +--+
+
+
+properties:
+ compatible:
+const: starfive,display-subsystem
+
+ clocks:
+items:
+ - description: Clock for display system noc bus.
+ - description: Core clock for display controller.
+ - description: Clock for axi bus to access ddr.
+ - description: Clock for ahb bus to R/W the phy regs.
+
+ clock-names:
+items:
+ - const: noc_bus
+ - const: dc_core
+ - const: axi_core
+ - const: ahb
+
+ resets:
+items:
+ - description: Reset for axi bus.
+ - description: Reset for ahb bus.
+ - description: Core reset of display controller.
+
+ reset-names:
+items:
+ - const: axi
+ - const: ahb
+ - const: core
+
+ ports:
+$ref: /schemas/types.yaml#/definitions/phandle-array
+items:
+ maxItems: 1
+description:
+ Should contain a list of phandles pointing to display interface port
+ of dc-controller devices.
+
+required:
+ - compatible
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+display-subsystem {
+compatible = "starfive,display-subsystem";
+ports = <&dc_out>;
+
+clocks = <&syscrg 60>,
+ <&voutcrg 4>,
+ <&voutcrg 5>,
+ <&voutcrg 6>;
+clock-names = "noc_bus", "dc_core", "axi_core", "ahb";
+resets = <&voutcrg 0>, <&voutcrg 1>, <&voutcrg 2>;
+reset-names = "axi", "ahb", "core";
+};
diff --git
a/Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml
b/Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml
new file mode 100644
index ..2cc0ad8e65ba
--- /dev/null
+++
b/Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id:
[v3 6/6] drm/vs: simple encoder
add simple encoder for dsi bridge
Signed-off-by: Keith Zhao
---
drivers/gpu/drm/verisilicon/Makefile| 4 +-
drivers/gpu/drm/verisilicon/vs_drv.c| 2 +
drivers/gpu/drm/verisilicon/vs_simple_enc.c | 195
drivers/gpu/drm/verisilicon/vs_simple_enc.h | 23 +++
4 files changed, 223 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/verisilicon/vs_simple_enc.c
create mode 100644 drivers/gpu/drm/verisilicon/vs_simple_enc.h
diff --git a/drivers/gpu/drm/verisilicon/Makefile
b/drivers/gpu/drm/verisilicon/Makefile
index 71fadafcee13..cd5d0a90bcfe 100644
--- a/drivers/gpu/drm/verisilicon/Makefile
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -5,6 +5,8 @@ vs_drm-objs := vs_dc_hw.o \
vs_crtc.o \
vs_drv.o \
vs_modeset.o \
- vs_plane.o
+ vs_plane.o \
+ vs_simple_enc.o
+
vs_drm-$(CONFIG_DRM_VERISILICON_STARFIVE_HDMI) += starfive_hdmi.o
obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
diff --git a/drivers/gpu/drm/verisilicon/vs_drv.c
b/drivers/gpu/drm/verisilicon/vs_drv.c
index d7e5199fe293..946f137ab124 100644
--- a/drivers/gpu/drm/verisilicon/vs_drv.c
+++ b/drivers/gpu/drm/verisilicon/vs_drv.c
@@ -23,6 +23,7 @@
#include "vs_drv.h"
#include "vs_modeset.h"
#include "vs_dc.h"
+#include "vs_simple_enc.h"
#define DRV_NAME "verisilicon"
#define DRV_DESC "Verisilicon DRM driver"
@@ -217,6 +218,7 @@ static struct platform_driver *drm_sub_drivers[] = {
#ifdef CONFIG_DRM_VERISILICON_STARFIVE_HDMI
&starfive_hdmi_driver,
#endif
+ &simple_encoder_driver,
};
static struct component_match *vs_drm_match_add(struct device *dev)
diff --git a/drivers/gpu/drm/verisilicon/vs_simple_enc.c
b/drivers/gpu/drm/verisilicon/vs_simple_enc.c
new file mode 100644
index ..c5a8d82bc469
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_simple_enc.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 VeriSilicon Holdings Co., Ltd.
+ */
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "vs_crtc.h"
+#include "vs_simple_enc.h"
+
+static const struct simple_encoder_priv dsi_priv = {
+ .encoder_type = DRM_MODE_ENCODER_DSI
+};
+
+static inline struct simple_encoder *to_simple_encoder(struct drm_encoder *enc)
+{
+ return container_of(enc, struct simple_encoder, encoder);
+}
+
+static int encoder_parse_dt(struct device *dev)
+{
+ struct simple_encoder *simple = dev_get_drvdata(dev);
+ unsigned int args[2];
+
+ simple->dss_regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
+
"starfive,syscon",
+ 2, args);
+
+ if (IS_ERR(simple->dss_regmap)) {
+ return dev_err_probe(dev, PTR_ERR(simple->dss_regmap),
+"getting the regmap failed\n");
+ }
+
+ simple->offset = args[0];
+ simple->mask = args[1];
+
+ return 0;
+}
+
+void encoder_atomic_enable(struct drm_encoder *encoder,
+ struct drm_atomic_state *state)
+{
+ struct simple_encoder *simple = to_simple_encoder(encoder);
+
+ regmap_update_bits(simple->dss_regmap, simple->offset, simple->mask,
+ simple->mask);
+}
+
+int encoder_atomic_check(struct drm_encoder *encoder,
+struct drm_crtc_state *crtc_state,
+struct drm_connector_state *conn_state)
+{
+ struct vs_crtc_state *vs_crtc_state = to_vs_crtc_state(crtc_state);
+ struct drm_connector *connector = conn_state->connector;
+ int ret = 0;
+
+ struct drm_bridge *first_bridge =
drm_bridge_chain_get_first_bridge(encoder);
+ struct drm_bridge_state *bridge_state = ERR_PTR(-EINVAL);
+
+ vs_crtc_state->encoder_type = encoder->encoder_type;
+
+ if (first_bridge && first_bridge->funcs->atomic_duplicate_state)
+ bridge_state = drm_atomic_get_bridge_state(crtc_state->state,
first_bridge);
+
+ if (IS_ERR(bridge_state)) {
+ if (connector->display_info.num_bus_formats)
+ vs_crtc_state->output_fmt =
connector->display_info.bus_formats[0];
+ else
+ vs_crtc_state->output_fmt = MEDIA_BUS_FMT_FIXED;
+ } else {
+ vs_crtc_state->output_fmt = bridge_state->input_bus_cfg.format;
+ }
+
+ switch (vs_crtc_state->output_fmt) {
+ case MEDIA_BUS_FMT_FIXED:
+ case MEDIA_BUS_FMT_RGB565_1X16:
+ case MEDIA_BUS_FMT_RGB666_1X18:
+ case MEDIA_BUS_FMT_RGB888_1X24:
+ case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+ case MEDIA_BUS_FMT_RGB101010_1X30:
+ case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
+ case MEDIA_BUS_FMT_UYVY8_1X16:
+ case MEDIA_BUS_FMT_YUV8_1X24:
+
[v3 5/6] drm/vs: Add hdmi driver
add hdmi driver as encoder and connect
Signed-off-by: Keith Zhao
---
drivers/gpu/drm/verisilicon/Kconfig | 8 +
drivers/gpu/drm/verisilicon/Makefile| 1 +
drivers/gpu/drm/verisilicon/starfive_hdmi.c | 849
drivers/gpu/drm/verisilicon/starfive_hdmi.h | 304 +++
drivers/gpu/drm/verisilicon/vs_drv.c| 3 +
drivers/gpu/drm/verisilicon/vs_drv.h| 4 +
6 files changed, 1169 insertions(+)
create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.c
create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.h
diff --git a/drivers/gpu/drm/verisilicon/Kconfig
b/drivers/gpu/drm/verisilicon/Kconfig
index e10fa97635aa..122c786e3948 100644
--- a/drivers/gpu/drm/verisilicon/Kconfig
+++ b/drivers/gpu/drm/verisilicon/Kconfig
@@ -11,3 +11,11 @@ config DRM_VERISILICON
This driver provides VeriSilicon kernel mode
setting and buffer management. It does not
provide 2D or 3D acceleration.
+
+config DRM_VERISILICON_STARFIVE_HDMI
+ bool "Starfive HDMI extensions"
+ depends on DRM_VERISILICON
+ help
+ This selects support for StarFive soc specific extensions
+ for the Innosilicon HDMI driver. If you want to enable
+ HDMI on JH7110 based soc, you should select this option.
diff --git a/drivers/gpu/drm/verisilicon/Makefile
b/drivers/gpu/drm/verisilicon/Makefile
index bf6f2b7ee480..71fadafcee13 100644
--- a/drivers/gpu/drm/verisilicon/Makefile
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -6,4 +6,5 @@ vs_drm-objs := vs_dc_hw.o \
vs_drv.o \
vs_modeset.o \
vs_plane.o
+vs_drm-$(CONFIG_DRM_VERISILICON_STARFIVE_HDMI) += starfive_hdmi.o
obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
diff --git a/drivers/gpu/drm/verisilicon/starfive_hdmi.c
b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
new file mode 100644
index ..aa621db0dee0
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
@@ -0,0 +1,849 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 StarFive Technology Co., Ltd.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "starfive_hdmi.h"
+#include "vs_drv.h"
+#include "vs_crtc.h"
+
+static const char * const hdmi_clocks[] = {
+ "sysclk",
+ "mclk",
+ "bclk"
+};
+
+static struct starfive_hdmi_encoder *encoder_to_hdmi(struct drm_encoder
*encoder)
+{
+ return container_of(encoder, struct starfive_hdmi_encoder, encoder);
+}
+
+static struct starfive_hdmi *connector_to_hdmi(struct drm_connector *connector)
+{
+ return container_of(connector, struct starfive_hdmi, connector);
+}
+
+static const struct post_pll_config post_pll_cfg_table[] = {
+ {2520, 1, 80, 13, 3, 1},
+ {2700, 1, 40, 11, 3, 1},
+ {3375, 1, 40, 11, 3, 1},
+ {4900, 1, 20, 1, 3, 3},
+ {24170, 1, 20, 1, 3, 3},
+ {29700, 4, 20, 0, 0, 3},
+ {59400, 4, 20, 0, 0, 0},
+ { /* sentinel */ }
+};
+
+inline u8 hdmi_readb(struct starfive_hdmi *hdmi, u16 offset)
+{
+ return readl_relaxed(hdmi->regs + (offset) * 0x04);
+}
+
+inline void hdmi_writeb(struct starfive_hdmi *hdmi, u16 offset, u32 val)
+{
+ writel_relaxed(val, hdmi->regs + (offset) * 0x04);
+}
+
+inline void hdmi_writew(struct starfive_hdmi *hdmi, u16 offset, u32 val)
+{
+ writew_relaxed(val & 0xFF, hdmi->regs + (offset) * 0x04);
+ writew_relaxed((val >> 8) & 0xFF, hdmi->regs + (offset + 1) * 0x04);
+}
+
+inline void hdmi_modb(struct starfive_hdmi *hdmi, u16 offset,
+u32 msk, u32 val)
+{
+ u8 temp = hdmi_readb(hdmi, offset) & ~msk;
+
+ temp |= val & msk;
+ hdmi_writeb(hdmi, offset, temp);
+}
+
+static int starfive_hdmi_enable_clk_deassert_rst(struct device *dev, struct
starfive_hdmi *hdmi)
+{
+ int ret;
+
+ ret = clk_bulk_prepare_enable(hdmi->nclks, hdmi->clk_hdmi);
+ if (ret) {
+ dev_err(dev, "failed to enable clocks\n");
+ return ret;
+ }
+
+ ret = reset_control_deassert(hdmi->tx_rst);
+ if (ret < 0) {
+ dev_err(dev, "failed to deassert tx_rst\n");
+ return ret;
+ }
+ return 0;
+}
+
+static void starfive_hdmi_disable_clk_assert_rst(struct device *dev, struct
starfive_hdmi *hdmi)
+{
+ int ret;
+
+ ret = reset_control_assert(hdmi->tx_rst);
+ if (ret < 0)
+ dev_err(dev, "failed to assert tx_rst\n");
+
+ clk_bulk_disable_unprepare(hdmi->nclks, hdmi->clk_hdmi);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int hdmi_system_pm_suspend(struct device *dev)
+{
+ return pm_runtime_force_suspend(dev);
+}
+
+static int hdmi_system_pm_resume(struct device *dev)
+{
+ return pm_ru
[v3 3/6] drm/vs: Register DRM device
Implement drm device registration interface
Signed-off-by: Keith Zhao
---
MAINTAINERS | 1 +
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/verisilicon/Kconfig | 13 +
drivers/gpu/drm/verisilicon/Makefile | 6 +
drivers/gpu/drm/verisilicon/vs_drv.c | 316 +++
drivers/gpu/drm/verisilicon/vs_drv.h | 42 +++
drivers/gpu/drm/verisilicon/vs_modeset.c | 39 +++
drivers/gpu/drm/verisilicon/vs_modeset.h | 10 +
9 files changed, 430 insertions(+)
create mode 100644 drivers/gpu/drm/verisilicon/Kconfig
create mode 100644 drivers/gpu/drm/verisilicon/Makefile
create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.c
create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.h
create mode 100644 drivers/gpu/drm/verisilicon/vs_modeset.c
create mode 100644 drivers/gpu/drm/verisilicon/vs_modeset.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 7caaadb83f3f..8dc9ebfe4605 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6887,6 +6887,7 @@ L:dri-devel@lists.freedesktop.org
S: Maintained
T: git git://anongit.freedesktop.org/drm/drm-misc
F: Documentation/devicetree/bindings/display/starfive/
+F: drivers/gpu/drm/verisilicon/
DRM DRIVER FOR TI DLPC3433 MIPI DSI TO DMD BRIDGE
M: Jagan Teki
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 3eee8636f847..e8d53c2e7c86 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -394,6 +394,8 @@ source "drivers/gpu/drm/solomon/Kconfig"
source "drivers/gpu/drm/sprd/Kconfig"
+source "drivers/gpu/drm/verisilicon/Kconfig"
+
config DRM_HYPERV
tristate "DRM Support for Hyper-V synthetic video device"
depends on DRM && PCI && MMU && HYPERV
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 8e1bde059170..29e04acded06 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -198,3 +198,4 @@ obj-$(CONFIG_DRM_HYPERV) += hyperv/
obj-y += solomon/
obj-$(CONFIG_DRM_SPRD) += sprd/
obj-$(CONFIG_DRM_LOONGSON) += loongson/
+obj-$(CONFIG_DRM_VERISILICON) += verisilicon/
diff --git a/drivers/gpu/drm/verisilicon/Kconfig
b/drivers/gpu/drm/verisilicon/Kconfig
new file mode 100644
index ..e10fa97635aa
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+config DRM_VERISILICON
+ tristate "DRM Support for VeriSilicon"
+ depends on DRM
+ select DRM_KMS_HELPER
+ select DRM_GEM_DMA_HELPER
+ select CMA
+ select DMA_CMA
+ help
+ Choose this option if you have a VeriSilicon soc chipset.
+ This driver provides VeriSilicon kernel mode
+ setting and buffer management. It does not
+ provide 2D or 3D acceleration.
diff --git a/drivers/gpu/drm/verisilicon/Makefile
b/drivers/gpu/drm/verisilicon/Makefile
new file mode 100644
index ..d785a1dfaa7f
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+
+vs_drm-objs := vs_drv.o \
+ vs_modeset.o
+
+obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
diff --git a/drivers/gpu/drm/verisilicon/vs_drv.c
b/drivers/gpu/drm/verisilicon/vs_drv.c
new file mode 100644
index ..4fb1f29ef84b
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_drv.c
@@ -0,0 +1,316 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
+ */
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "vs_drv.h"
+#include "vs_modeset.h"
+
+#define DRV_NAME "verisilicon"
+#define DRV_DESC "Verisilicon DRM driver"
+#define DRV_DATE "20230516"
+#define DRV_MAJOR 1
+#define DRV_MINOR 0
+
+static int vs_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
+ struct drm_mode_create_dumb *args)
+{
+ struct vs_drm_device *priv = to_vs_drm_private(dev);
+ unsigned int pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
+
+ args->pitch = ALIGN(pitch, priv->pitch_alignment);
+ return drm_gem_dma_dumb_create_internal(file, dev, args);
+}
+
+DEFINE_DRM_GEM_FOPS(vs_drm_fops);
+
+static struct drm_driver vs_drm_driver = {
+ .driver_features= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
+
+ DRM_GEM_DMA_DRIVER_OPS_WITH_DUMB_CREATE(vs_gem_dumb_create),
+
+ .fops = &vs_drm_fops,
+ .name = DRV_NAME,
+ .desc = DRV_DESC,
+ .date = DRV_DATE,
+ .major = DRV_MAJOR,
+ .minor = DRV_MINOR,
+};
+
+static void vs_drm_device_release_clocks(void *res)
+{
+ struct vs_drm_device *priv = res;
+ unsigned int i;
+
+ r
dri-devel@lists.freedesktop.org
add 2 crtcs and 8 planes in vs-drm
Signed-off-by: Keith Zhao
---
drivers/gpu/drm/verisilicon/Makefile |9 +-
drivers/gpu/drm/verisilicon/vs_crtc.c | 208 +
drivers/gpu/drm/verisilicon/vs_crtc.h | 42 +
drivers/gpu/drm/verisilicon/vs_dc.c| 1192
drivers/gpu/drm/verisilicon/vs_dc.h| 67 ++
drivers/gpu/drm/verisilicon/vs_dc_hw.c | 1022
drivers/gpu/drm/verisilicon/vs_dc_hw.h | 580
drivers/gpu/drm/verisilicon/vs_drv.c |2 +
drivers/gpu/drm/verisilicon/vs_plane.c | 301 ++
drivers/gpu/drm/verisilicon/vs_plane.h | 39 +
drivers/gpu/drm/verisilicon/vs_type.h | 69 ++
11 files changed, 3528 insertions(+), 3 deletions(-)
create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.c
create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.h
create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.c
create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.h
create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.c
create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.h
create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.c
create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.h
create mode 100644 drivers/gpu/drm/verisilicon/vs_type.h
diff --git a/drivers/gpu/drm/verisilicon/Makefile
b/drivers/gpu/drm/verisilicon/Makefile
index d785a1dfaa7f..bf6f2b7ee480 100644
--- a/drivers/gpu/drm/verisilicon/Makefile
+++ b/drivers/gpu/drm/verisilicon/Makefile
@@ -1,6 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
-vs_drm-objs := vs_drv.o \
- vs_modeset.o
-
+vs_drm-objs := vs_dc_hw.o \
+ vs_dc.o \
+ vs_crtc.o \
+ vs_drv.o \
+ vs_modeset.o \
+ vs_plane.o
obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c
b/drivers/gpu/drm/verisilicon/vs_crtc.c
new file mode 100644
index ..5581219b1230
--- /dev/null
+++ b/drivers/gpu/drm/verisilicon/vs_crtc.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 VeriSilicon Holdings Co., Ltd.
+ *
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#include "vs_crtc.h"
+#include "vs_dc.h"
+#include "vs_drv.h"
+
+static void vs_crtc_atomic_destroy_state(struct drm_crtc *crtc,
+struct drm_crtc_state *state)
+{
+ __drm_atomic_helper_crtc_destroy_state(state);
+ kfree(to_vs_crtc_state(state));
+}
+
+static void vs_crtc_reset(struct drm_crtc *crtc)
+{
+ struct vs_crtc_state *state;
+
+ if (crtc->state)
+ vs_crtc_atomic_destroy_state(crtc, crtc->state);
+
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return;
+
+ __drm_atomic_helper_crtc_reset(crtc, &state->base);
+}
+
+static struct drm_crtc_state *
+vs_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
+{
+ struct vs_crtc_state *old_state;
+ struct vs_crtc_state *state;
+
+ if (!crtc->state)
+ return NULL;
+
+ old_state = to_vs_crtc_state(crtc->state);
+
+ state = kmemdup(old_state, sizeof(*old_state), GFP_KERNEL);
+ if (!state)
+ return NULL;
+
+ __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base);
+
+ return &state->base;
+}
+
+static int vs_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+ struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
+ struct vs_dc *dc = dev_get_drvdata(vs_crtc->dev);
+
+ vs_dc_enable_vblank(dc, true);
+
+ return 0;
+}
+
+static void vs_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+ struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
+ struct vs_dc *dc = dev_get_drvdata(vs_crtc->dev);
+
+ vs_dc_enable_vblank(dc, false);
+}
+
+static void vs_crtc_atomic_print_state(struct drm_printer *p,
+ const struct drm_crtc_state *state)
+{
+ struct vs_crtc *vs_crtc = to_vs_crtc(state->crtc);
+
+ drm_printf(p, "vs crtc State\n");
+ drm_printf(p, "\tcolor_formats: %d\n", vs_crtc->color_formats);
+ drm_printf(p, "\tmax_bpc: %d\n", vs_crtc->max_bpc);
+}
+
+static const struct drm_crtc_funcs vs_crtc_funcs = {
+ .set_config = drm_atomic_helper_set_config,
+ .page_flip = drm_atomic_helper_page_flip,
+ .reset = vs_crtc_reset,
+ .atomic_duplicate_state = vs_crtc_atomic_duplicate_state,
+ .atomic_destroy_state = vs_crtc_atomic_destroy_state,
+ .enable_vblank = vs_crtc_enable_vblank,
+ .disable_vblank = vs_crtc_disable_vblank,
+ .atomic_print_state = vs_crtc_atomic_print_state,
+};
+
+static void vs_crtc_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
+ struct vs_dc *dc = dev_get_drvdata(vs_crtc->dev);
+
+
[v3 0/6] DRM driver for verisilicon
This patch is a drm driver for Starfive Soc JH7110, I am sending Drm driver part and HDMI driver part. We used GEM framework for buffer management, and for buffer allocation,we use DMA APIs. the Starfive HDMI servers as interface between a LCD Controller and a HDMI bus. A HDMI TX consists of one HDMI transmitter controller and one HDMI transmitter PHY. (Sound support is not include in this patch) This patchset should be applied on next branch. V1: Changes since v1: - Further standardize the yaml file. - Dts naming convention improved. - Fix the problem of compiling and loading ko files. - Use drm new api to automatically manage resources. - Drop vs_crtc_funcs&vs_plane_funcs, subdivide the plane's help interface. - Reduce the modifiers unused. - Optimize the hdmi driver code V2: Changes since v2: - fix the error about checking the yaml file. - match drm driver GEM DMA API. - Delete the custom crtc property . - hdmi use drmm_ new api to automatically manage resources. - update the modifiers comments. - enabling KASAN, fix the error during removing module V3: Changes since v3: - Delete the custom plane property. - Delete the custom fourcc modifiers. - Adjust the calculation mode of hdmi pixclock. - Add match data for dc8200 driver. - Adjust some magic values. - Add a simple encoder for dsi output. Keith Zhao (6): dt-bindings: display: Add yamls for JH7110 display system riscv: dts: starfive: jh7110: display subsystem drm/vs: Register DRM device drm/vs: Add KMS crtc&plane drm/vs: Add hdmi driver drm/vs: simple encoder .../starfive/starfive,display-subsystem.yaml | 104 ++ .../starfive/starfive,dsi-encoder.yaml| 92 ++ .../starfive/starfive,jh7110-dc8200.yaml | 113 ++ .../starfive/starfive,jh7110-inno-hdmi.yaml | 82 ++ .../soc/starfive/starfive,jh7110-syscon.yaml |1 + MAINTAINERS |8 + .../jh7110-starfive-visionfive-2.dtsi | 134 ++ arch/riscv/boot/dts/starfive/jh7110.dtsi | 49 + drivers/gpu/drm/Kconfig |2 + drivers/gpu/drm/Makefile |1 + drivers/gpu/drm/verisilicon/Kconfig | 21 + drivers/gpu/drm/verisilicon/Makefile | 12 + drivers/gpu/drm/verisilicon/starfive_hdmi.c | 849 drivers/gpu/drm/verisilicon/starfive_hdmi.h | 304 + drivers/gpu/drm/verisilicon/vs_crtc.c | 208 +++ drivers/gpu/drm/verisilicon/vs_crtc.h | 42 + drivers/gpu/drm/verisilicon/vs_dc.c | 1192 + drivers/gpu/drm/verisilicon/vs_dc.h | 67 + drivers/gpu/drm/verisilicon/vs_dc_hw.c| 1022 ++ drivers/gpu/drm/verisilicon/vs_dc_hw.h| 580 drivers/gpu/drm/verisilicon/vs_drv.c | 323 + drivers/gpu/drm/verisilicon/vs_drv.h | 46 + drivers/gpu/drm/verisilicon/vs_modeset.c | 39 + drivers/gpu/drm/verisilicon/vs_modeset.h | 10 + drivers/gpu/drm/verisilicon/vs_plane.c| 301 + drivers/gpu/drm/verisilicon/vs_plane.h| 39 + drivers/gpu/drm/verisilicon/vs_simple_enc.c | 195 +++ drivers/gpu/drm/verisilicon/vs_simple_enc.h | 23 + drivers/gpu/drm/verisilicon/vs_type.h | 69 + 29 files changed, 5928 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml create mode 100644 drivers/gpu/drm/verisilicon/Kconfig create mode 100644 drivers/gpu/drm/verisilicon/Makefile create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.c create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.h create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.c create mode 100644 drivers/gpu/drm/verisilicon/vs_crtc.h create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.c create mode 100644 drivers/gpu/drm/verisilicon/vs_dc.h create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.c create mode 100644 drivers/gpu/drm/verisilicon/vs_dc_hw.h create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.c create mode 100644 drivers/gpu/drm/verisilicon/vs_drv.h create mode 100644 drivers/gpu/drm/verisilicon/vs_modeset.c create mode 100644 drivers/gpu/drm/verisilicon/vs_modeset.h create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.c create mode 100644 drivers/gpu/drm/verisilicon/vs_plane.h create mode 100644 drivers/gpu/drm/verisilicon/vs_simple_enc.c create mode 100644 drivers/gpu/drm/verisilicon/vs_simple_enc.h create mode 100644 drivers/gpu/drm/verisilicon/vs_type.h -- 2.34.1
[bug report] drm/amdgpu: Workaround to skip kiq ring test during ras gpu recovery
Hello Stanley.Yang,
The patch b1338a8e71ac: "drm/amdgpu: Workaround to skip kiq ring test
during ras gpu recovery" from Oct 17, 2023 (linux-next), leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c:604 amdgpu_get_xgmi_hive()
warn: sleeping in atomic context
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
591 struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device
*adev)
592 {
593 struct amdgpu_hive_info *hive = NULL;
594 int ret;
595
596 if (!adev->gmc.xgmi.hive_id)
597 return NULL;
598
599 if (adev->hive) {
600 kobject_get(&adev->hive->kobj);
601 return adev->hive;
602 }
603
--> 604 mutex_lock(&xgmi_mutex);
^^^
Shhh The mutexes are sleeping.
605
606 list_for_each_entry(hive, &xgmi_hive_list, node) {
The caller is amdgpu_gfx_disable_kcq():
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
516 spin_lock(&kiq->ring_lock);
^^^
Holding a spin lock.
517 if (amdgpu_ring_alloc(kiq_ring, kiq->pmf->unmap_queues_size *
518 adev->gfx.num_compute_rings)) {
519 spin_unlock(&kiq->ring_lock);
520 return -ENOMEM;
521 }
522
523 for (i = 0; i < adev->gfx.num_compute_rings; i++) {
524 j = i + xcc_id * adev->gfx.num_compute_rings;
525 kiq->pmf->kiq_unmap_queues(kiq_ring,
526 &adev->gfx.compute_ring[j],
527 RESET_QUEUES, 0, 0);
528 }
529
530 /**
531 * This is workaround: only skip kiq_ring test
532 * during ras recovery in suspend stage for gfx9.4.3
533 */
534 hive = amdgpu_get_xgmi_hive(adev);
^^
Can't call a sleeping function when holding a spin_lock.
535 if (hive) {
536 hive_ras_recovery = atomic_read(&hive->ras_recovery);
537 amdgpu_put_xgmi_hive(hive);
538 }
539
540 ras = amdgpu_ras_get_context(adev);
541 if ((amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4,
3)) &&
542 ras && (atomic_read(&ras->in_recovery) ||
hive_ras_recovery)) {
543 spin_unlock(&kiq->ring_lock);
544 return 0;
545 }
546
547 if (kiq_ring->sched.ready && !adev->job_hang)
548 r = amdgpu_ring_test_helper(kiq_ring);
549 spin_unlock(&kiq->ring_lock);
regards,
dan carpenter
Re: [PATCH v18 04/26] drm/shmem-helper: Refactor locked/unlocked functions
On Wed, Nov 29, 2023 at 04:47:05PM +0100, Boris Brezillon wrote:
> On Wed, 29 Nov 2023 16:15:27 +0100
> Maxime Ripard wrote:
>
> > > Now, let's assume we drop the _locked() suffix on
> > > drm_gem_shmem_v[un]map(), but keep it on other helpers that need both
> > > variants. This results in an inconsistent naming scheme inside the
> > > same source file, which I find utterly confusing.
> > >
> > > Note that the initial reason I asked Dmitry if he could add the
> > > _locked suffix to drm_gem_shmem_vmap() is because I started using
> > > drm_gem_shmem_vmap() in powervr, before realizing this version wasn't
> > > taking the lock, and I should have used drm_gem_vmap_unlocked()
> > > instead, so this is not something I'm making up.
> >
> > Sorry if I gave you the impression I thought that you're making that up,
> > I'm not.
> >
> > Thanks for the explanation btw, I think I get what you're saying now:
> >
> > - drm_gem_shmem_vmap() is never taking the locks because the core
> >expects to take them before calling them.
> >
> > - drm_gem_shmem_vunmap() is never taking the locks because the core
> >expects to take them before calling them.
>
> Correct.
>
> >
> > - Some other code path can still call those helpers in drivers, and the
> >locking isn't handled by the core anymore.
>
> They can, if they want to v[un]map a BO and they already acquired the
> GEM resv lock. But I'm not sure anyone needs to do that yet. The main
> reason for exposing these helpers is if one driver needs to overload the
> default gem_shmem_funcs.
>
> >
> > - We now have _vmap/vunmap_unlocked functions to take those locks for
> >those code paths
>
> We don't have drm_gem_shmem_vmap/vunmap_unlocked(), we have
> drm_gem_shmem_vmap/vunmap_locked(), which can be called directly, but
> are mainly used to populate the drm_gem_object_funcs vtable. If drivers
> want to v[un]map in a path where the resv lock is not held, they should
> call drm_gem_vmap/vunmap_unlocked() (which are renamed
> drm_gem_vmap/vunmap() in patch 1 of this series). Mind the **drm_gem_**
> vs **drm_gem_shmem_** difference in the helper names. drm_gem_ helpers
> are provided by drm_gem.c and call drm_gem_object_funcs callback, which
> are supposed to be populated with drm_gem_shmem helpers.
>
> >
> > - And the variant names are now confusing, making people use the
> >lockless version in situations where they should have use the locked
> >one.
>
> That's what happened to me, at least.
>
> >
> > Is that a correct summary?
>
> Almost ;-).
>
> >
> > If so, then I agree that we need to change the name.
>
> Cool.
>
> >
> > We discussed it some more on IRC, and we agree that the "default"
> > function should handle the locking properly and that's what the most
> > common case should use.
>
> Agree if by 'default' you mean the lock is always acquired by the
> helper, not 'let's decide based on what users do most of the time with
> this specific helper', because otherwise we'd be back to a situation
> where the name doesn't clearly encode the function behavior.
>
> >
> > So that means than drm_gem_shmem_vmap/vunmap() should take the lock
> > itself, and drm_gem_shmem_vmap/vunmap_nolock/unlocked never does.
>
> Not sure we have a need for drm_gem_shmem_vmap/vunmap(), but if we ever
> add such helpers, they would acquire the resv lock, indeed.
>
> Just to be clear, _nolock == _locked in the current semantics :-).
> _nolock means 'don't take the lock', and _locked means 'lock is already
> held'.
>
> >
> > I think I'd prefer the nolock variant over unlocked still.
>
> Okay, that means s/_locked/_nolock/ in drm_gem_shmem_helpers.{c,h}, I
> guess.
>
> >
> > And I also think we can improve the documentation and add lockdep calls
>
> Lockdep asserts are already there, I think.
>
> > to make sure that the difference between variants is clear in the doc,
> > and if someone still get confused we can catch it.
> >
> > Does that sound like a plan?
>
> Assuming I understood it correctly, yes. Can you just confirm my
> understanding is correct though?
We are. Sorry for delaying this :)
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v1 3/3] drm/bridge: lt8912b: Add power supplies
On Wed, Nov 15, 2023 at 1:14 PM Francesco Dolcini wrote:
>
> From: Stefan Eichenberger
>
> Add supplies to the driver that can be used to turn the Lontium lt8912b
> on and off. It can have up to 7 independent supplies, we add them all
> and enable/disable them with bulk_enable/disable.
>
> Signed-off-by: Stefan Eichenberger
> Signed-off-by: Francesco Dolcini
> ---
> drivers/gpu/drm/bridge/lontium-lt8912b.c | 30
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> index 097ab04234b7..273157428c82 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> @@ -43,6 +43,8 @@ struct lt8912 {
>
> struct videomode mode;
>
> + struct regulator_bulk_data supplies[7];
> +
> u8 data_lanes;
> bool is_power_on;
> };
> @@ -257,6 +259,12 @@ static int lt8912_free_i2c(struct lt8912 *lt)
>
> static int lt8912_hard_power_on(struct lt8912 *lt)
> {
> + int ret;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(lt->supplies), lt->supplies);
> + if (ret)
> + return ret;
> +
> gpiod_set_value_cansleep(lt->gp_reset, 0);
> msleep(20);
>
> @@ -267,6 +275,9 @@ static void lt8912_hard_power_off(struct lt8912 *lt)
> {
> gpiod_set_value_cansleep(lt->gp_reset, 1);
> msleep(20);
> +
> + regulator_bulk_disable(ARRAY_SIZE(lt->supplies), lt->supplies);
> +
> lt->is_power_on = false;
> }
>
> @@ -661,6 +672,21 @@ static int lt8912_bridge_suspend(struct device *dev)
>
> static DEFINE_SIMPLE_DEV_PM_OPS(lt8912_bridge_pm_ops, lt8912_bridge_suspend,
> lt8912_bridge_resume);
>
> +static int lt8912_get_regulators(struct lt8912 *lt)
> +{
> + unsigned int i;
> + const char * const supply_names[] = {
> + "vdd", "vccmipirx", "vccsysclk", "vcclvdstx",
> + "vcchdmitx", "vcclvdspll", "vcchdmipll"
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(lt->supplies); i++)
> + lt->supplies[i].supply = supply_names[i];
> +
> + return devm_regulator_bulk_get(lt->dev, ARRAY_SIZE(lt->supplies),
> + lt->supplies);
> +}
> +
> static int lt8912_parse_dt(struct lt8912 *lt)
> {
> struct gpio_desc *gp_reset;
> @@ -712,6 +738,10 @@ static int lt8912_parse_dt(struct lt8912 *lt)
> goto err_free_host_node;
> }
>
> + ret = lt8912_get_regulators(lt);
> + if (ret)
> + goto err_free_host_node;
> +
> of_node_put(port_node);
> return 0;
>
> --
> 2.25.1
>
Reviewed-by: Robert Foss
Re: [PATCH v1 1/3] drm/bridge: lt8912b: Add suspend/resume support
On Wed, Nov 15, 2023 at 1:14 PM Francesco Dolcini wrote:
>
> From: Stefan Eichenberger
>
> Add support for suspend and resume. The lt8912b will power off when
> going into suspend and power on when resuming.
>
> Signed-off-by: Stefan Eichenberger
> Signed-off-by: Francesco Dolcini
> ---
> drivers/gpu/drm/bridge/lontium-lt8912b.c | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> index 03532efb893b..097ab04234b7 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> @@ -634,6 +634,33 @@ static const struct drm_bridge_funcs lt8912_bridge_funcs
> = {
> .get_edid = lt8912_bridge_get_edid,
> };
>
> +static int lt8912_bridge_resume(struct device *dev)
> +{
> + struct lt8912 *lt = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = lt8912_hard_power_on(lt);
> + if (ret)
> + return ret;
> +
> + ret = lt8912_soft_power_on(lt);
> + if (ret)
> + return ret;
> +
> + return lt8912_video_on(lt);
> +}
> +
> +static int lt8912_bridge_suspend(struct device *dev)
> +{
> + struct lt8912 *lt = dev_get_drvdata(dev);
> +
> + lt8912_hard_power_off(lt);
> +
> + return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(lt8912_bridge_pm_ops, lt8912_bridge_suspend,
> lt8912_bridge_resume);
> +
> static int lt8912_parse_dt(struct lt8912 *lt)
> {
> struct gpio_desc *gp_reset;
> @@ -770,6 +797,7 @@ static struct i2c_driver lt8912_i2c_driver = {
> .driver = {
> .name = "lt8912",
> .of_match_table = lt8912_dt_match,
> + .pm = pm_sleep_ptr(<8912_bridge_pm_ops),
> },
> .probe = lt8912_probe,
> .remove = lt8912_remove,
> --
> 2.25.1
>
Reviewed-by: Robert Foss
[PATCH 0/7] drm: revert solid fill support
Altough the Solid Fill planes patchset got all reviews and acknowledgements, it doesn't fulfill requirements for the new uABI. Merging it was a fault of mine. It has neither corresponding open-source userspace implementation nor the IGT tests coverage. Revert this patchset until userspace obligations are fulfilled. Dmitry Baryshkov (7): Revert "drm/atomic: Loosen FB atomic checks" Revert "drm/atomic: Move framebuffer checks to helper" Revert "drm/atomic: Add solid fill data to plane state dump" Revert "drm/atomic: Add pixel source to plane state dump" Revert "drm: Add solid fill pixel source" Revert "drm: Introduce solid fill DRM plane property" Revert "drm: Introduce pixel_source DRM plane property" drivers/gpu/drm/drm_atomic.c | 148 +- drivers/gpu/drm/drm_atomic_helper.c | 39 +++--- drivers/gpu/drm/drm_atomic_state_helper.c | 10 -- drivers/gpu/drm/drm_atomic_uapi.c | 30 - drivers/gpu/drm/drm_blend.c | 133 --- drivers/gpu/drm/drm_crtc_internal.h | 1 - drivers/gpu/drm/drm_plane.c | 27 +--- include/drm/drm_atomic_helper.h | 4 +- include/drm/drm_blend.h | 3 - include/drm/drm_plane.h | 90 - include/uapi/drm/drm_mode.h | 24 11 files changed, 86 insertions(+), 423 deletions(-) -- 2.42.0
[PATCH 1/7] Revert "drm/atomic: Loosen FB atomic checks"
This reverts commit f1e75da5364e780905d9cd6043f9c74cdcf84073.
Altough the Solid Fill planes patchset got all reviews and
acknowledgements, it doesn't fulfill requirements for the new uABI. It
has neither corresponding open-source userspace implementation nor the
IGT tests coverage. Reverting this patchset until userspace obligations
are fulfilled.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_atomic.c| 21
drivers/gpu/drm/drm_atomic_helper.c | 39 +
include/drm/drm_atomic_helper.h | 4 +--
include/drm/drm_plane.h | 29 -
4 files changed, 29 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index aed0a694c74c..c6f2b86c48ae 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -674,16 +674,17 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
{
struct drm_plane *plane = new_plane_state->plane;
struct drm_crtc *crtc = new_plane_state->crtc;
+ const struct drm_framebuffer *fb = new_plane_state->fb;
int ret;
- /* either *both* CRTC and pixel source must be set, or neither */
- if (crtc && !drm_plane_has_visible_data(new_plane_state)) {
- drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no
visible data\n",
+ /* either *both* CRTC and FB must be set, or neither */
+ if (crtc && !fb) {
+ drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no FB\n",
plane->base.id, plane->name);
return -EINVAL;
- } else if (drm_plane_has_visible_data(new_plane_state) && !crtc) {
- drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] Source %d has visible
data but no CRTC\n",
- plane->base.id, plane->name,
new_plane_state->pixel_source);
+ } else if (fb && !crtc) {
+ drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] FB set but no CRTC\n",
+ plane->base.id, plane->name);
return -EINVAL;
}
@@ -714,11 +715,9 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
}
- if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB &&
new_plane_state->fb) {
- ret = drm_atomic_plane_check_fb(new_plane_state);
- if (ret)
- return ret;
- }
+ ret = drm_atomic_plane_check_fb(new_plane_state);
+ if (ret)
+ return ret;
if (plane_switching_crtc(old_plane_state, new_plane_state)) {
drm_dbg_atomic(plane->dev,
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index dc048988e3f3..c3f677130def 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -861,6 +861,7 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
bool can_position,
bool can_update_disabled)
{
+ struct drm_framebuffer *fb = plane_state->fb;
struct drm_rect *src = &plane_state->src;
struct drm_rect *dst = &plane_state->dst;
unsigned int rotation = plane_state->rotation;
@@ -872,7 +873,7 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
*src = drm_plane_state_src(plane_state);
*dst = drm_plane_state_dest(plane_state);
- if (!drm_plane_has_visible_data(plane_state)) {
+ if (!fb) {
plane_state->visible = false;
return 0;
}
@@ -889,31 +890,25 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
return -EINVAL;
}
- /* Check that selected pixel source is valid */
- if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB &&
plane_state->fb) {
- struct drm_framebuffer *fb = plane_state->fb;
- drm_rect_rotate(src, fb->width << 16, fb->height << 16,
rotation);
+ drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
- /* Check scaling */
- hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
- vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
+ /* Check scaling */
+ hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
+ vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
+ if (hscale < 0 || vscale < 0) {
+ drm_dbg_kms(plane_state->plane->dev,
+ "Invalid scaling of plane\n");
+ drm_rect_debug_print("src: ", &plane_state->src, true);
+ drm_rect_debug_print("dst: ", &plane_state->dst, false);
+ return -ERANGE;
+ }
- if (hscale < 0 || vscale < 0) {
-
[PATCH 2/7] Revert "drm/atomic: Move framebuffer checks to helper"
This reverts commit 4ba6b7a646321e740c7f2d80c90505019c4e8fce.
Altough the Solid Fill planes patchset got all reviews and
acknowledgements, it doesn't fulfill requirements for the new uABI. It
has neither corresponding open-source userspace implementation nor the
IGT tests coverage. Reverting this patchset until userspace obligations
are fulfilled.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_atomic.c | 130 +++
1 file changed, 57 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index c6f2b86c48ae..1339785fbe80 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -589,76 +589,6 @@ plane_switching_crtc(const struct drm_plane_state
*old_plane_state,
return true;
}
-static int drm_atomic_plane_check_fb(const struct drm_plane_state *state)
-{
- struct drm_plane *plane = state->plane;
- const struct drm_framebuffer *fb = state->fb;
- struct drm_mode_rect *clips;
-
- uint32_t num_clips;
- unsigned int fb_width, fb_height;
- int ret;
-
- /* Check whether this plane supports the fb pixel format. */
- ret = drm_plane_check_pixel_format(plane, fb->format->format,
- fb->modifier);
-
- if (ret) {
- drm_dbg_atomic(plane->dev,
- "[PLANE:%d:%s] invalid pixel format %p4cc,
modifier 0x%llx\n",
- plane->base.id, plane->name,
- &fb->format->format, fb->modifier);
- return ret;
- }
-
- fb_width = fb->width << 16;
- fb_height = fb->height << 16;
-
- /* Make sure source coordinates are inside the fb. */
- if (state->src_w > fb_width ||
- state->src_x > fb_width - state->src_w ||
- state->src_h > fb_height ||
- state->src_y > fb_height - state->src_h) {
- drm_dbg_atomic(plane->dev,
- "[PLANE:%d:%s] invalid source coordinates "
- "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
- plane->base.id, plane->name,
- state->src_w >> 16,
- ((state->src_w & 0x) * 15625) >> 10,
- state->src_h >> 16,
- ((state->src_h & 0x) * 15625) >> 10,
- state->src_x >> 16,
- ((state->src_x & 0x) * 15625) >> 10,
- state->src_y >> 16,
- ((state->src_y & 0x) * 15625) >> 10,
- fb->width, fb->height);
- return -ENOSPC;
- }
-
- clips = __drm_plane_get_damage_clips(state);
- num_clips = drm_plane_get_damage_clips_count(state);
-
- /* Make sure damage clips are valid and inside the fb. */
- while (num_clips > 0) {
- if (clips->x1 >= clips->x2 ||
- clips->y1 >= clips->y2 ||
- clips->x1 < 0 ||
- clips->y1 < 0 ||
- clips->x2 > fb_width ||
- clips->y2 > fb_height) {
- drm_dbg_atomic(plane->dev,
- "[PLANE:%d:%s] invalid damage clip %d %d
%d %d\n",
- plane->base.id, plane->name, clips->x1,
- clips->y1, clips->x2, clips->y2);
- return -EINVAL;
- }
- clips++;
- num_clips--;
- }
-
- return 0;
-}
-
/**
* drm_atomic_plane_check - check plane state
* @old_plane_state: old plane state to check
@@ -675,6 +605,9 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
struct drm_plane *plane = new_plane_state->plane;
struct drm_crtc *crtc = new_plane_state->crtc;
const struct drm_framebuffer *fb = new_plane_state->fb;
+ unsigned int fb_width, fb_height;
+ struct drm_mode_rect *clips;
+ uint32_t num_clips;
int ret;
/* either *both* CRTC and FB must be set, or neither */
@@ -701,6 +634,17 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
return -EINVAL;
}
+ /* Check whether this plane supports the fb pixel format. */
+ ret = drm_plane_check_pixel_format(plane, fb->format->format,
+ fb->modifier);
+ if (ret) {
+ drm_dbg_atomic(plane->dev,
+ "[PLANE:%d:%s] invalid pixel format %p4cc,
modifier 0x%llx\n",
+ plane->base.id, plane->name,
+ &fb->format->format, fb->modifier);
+ return ret;
+ }
+
/* Give drivers some help ag
[PATCH 4/7] Revert "drm/atomic: Add pixel source to plane state dump"
This reverts commit 8283ac7871a959848e09fc6593b8c12b8febfee6.
Altough the Solid Fill planes patchset got all reviews and
acknowledgements, it doesn't fulfill requirements for the new uABI. It
has neither corresponding open-source userspace implementation nor the
IGT tests coverage. Reverting this patchset until userspace obligations
are fulfilled.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_atomic.c| 1 -
drivers/gpu/drm/drm_blend.c | 1 -
drivers/gpu/drm/drm_crtc_internal.h | 1 -
3 files changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 02aa7df832cc..f1a503aafe5a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -722,7 +722,6 @@ static void drm_atomic_plane_print_state(struct drm_printer
*p,
drm_printf(p, "plane[%u]: %s\n", plane->base.id, plane->name);
drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
"(null)");
- drm_printf(p, "\tpixel-source=%s\n",
drm_get_pixel_source_name(state->pixel_source));
drm_printf(p, "\tfb=%u\n", state->fb ? state->fb->base.id : 0);
if (state->fb)
drm_framebuffer_print_info(p, 2, state->fb);
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 9c1608f7c1df..37b31b7e5ce5 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -647,7 +647,6 @@ static const struct drm_prop_enum_list
drm_pixel_source_enum_list[] = {
{ DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
{ DRM_PLANE_PIXEL_SOURCE_SOLID_FILL, "SOLID_FILL" },
};
-DRM_ENUM_NAME_FN(drm_get_pixel_source_name, drm_pixel_source_enum_list);
/**
* drm_plane_create_pixel_source_property - create a new pixel source property
diff --git a/drivers/gpu/drm/drm_crtc_internal.h
b/drivers/gpu/drm/drm_crtc_internal.h
index 4114675c0728..a514d5207e41 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -269,7 +269,6 @@ int drm_plane_check_pixel_format(struct drm_plane *plane,
u32 format, u64 modifier);
struct drm_mode_rect *
__drm_plane_get_damage_clips(const struct drm_plane_state *state);
-const char *drm_get_pixel_source_name(int val);
/* drm_bridge.c */
void drm_bridge_detach(struct drm_bridge *bridge);
--
2.42.0
[PATCH 7/7] Revert "drm: Introduce pixel_source DRM plane property"
This reverts commit e50e5fed41c7eed2db4119645bf3480ec43fec11.
Altough the Solid Fill planes patchset got all reviews and
acknowledgements, it doesn't fulfill requirements for the new uABI. It
has neither corresponding open-source userspace implementation nor the
IGT tests coverage. Reverting this patchset until userspace obligations
are fulfilled.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_atomic_state_helper.c | 1 -
drivers/gpu/drm/drm_atomic_uapi.c | 4 -
drivers/gpu/drm/drm_blend.c | 94 ---
drivers/gpu/drm/drm_plane.c | 19 +
include/drm/drm_blend.h | 2 -
include/drm/drm_plane.h | 21 -
6 files changed, 4 insertions(+), 137 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 311b04edf742..54975de44a0e 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -252,7 +252,6 @@ void __drm_atomic_helper_plane_state_reset(struct
drm_plane_state *plane_state,
plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
- plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
if (plane->color_encoding_property) {
if (!drm_object_property_get_default_value(&plane->base,
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index bd7140531948..aee4a65d4959 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -562,8 +562,6 @@ static int drm_atomic_plane_set_property(struct drm_plane
*plane,
state->src_w = val;
} else if (property == config->prop_src_h) {
state->src_h = val;
- } else if (property == plane->pixel_source_property) {
- state->pixel_source = val;
} else if (property == plane->alpha_property) {
state->alpha = val;
} else if (property == plane->blend_mode_property) {
@@ -652,8 +650,6 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_w;
} else if (property == config->prop_src_h) {
*val = state->src_h;
- } else if (property == plane->pixel_source_property) {
- *val = state->pixel_source;
} else if (property == plane->alpha_property) {
*val = state->alpha;
} else if (property == plane->blend_mode_property) {
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index fce734cdb85b..6e74de833466 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -185,25 +185,6 @@
* plane does not expose the "alpha" property, then this is
* assumed to be 1.0
*
- * pixel_source:
- * pixel_source is set up with drm_plane_create_pixel_source_property().
- * It is used to toggle the active source of pixel data for the plane.
- * The plane will only display data from the set pixel_source -- any
- * data from other sources will be ignored.
- *
- * For non-framebuffer sources, if pixel_source is set to a non-framebuffer
- * and non-NONE source, and the corresponding source property is NULL, then
- * the atomic commit should return an error.
- *
- * Possible values:
- *
- * "NONE":
- * No active pixel source.
- * Committing with a NONE pixel source will disable the plane.
- *
- * "FB":
- * Framebuffer source set by the "FB_ID" property.
- *
* Note that all the property extensions described here apply either to the
* plane or the CRTC (e.g. for the background color, which currently is not
* exposed and assumed to be black).
@@ -634,78 +615,3 @@ int drm_plane_create_blend_mode_property(struct drm_plane
*plane,
return 0;
}
EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
-
-static const struct drm_prop_enum_list drm_pixel_source_enum_list[] = {
- { DRM_PLANE_PIXEL_SOURCE_NONE, "NONE" },
- { DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
-};
-
-/**
- * drm_plane_create_pixel_source_property - create a new pixel source property
- * @plane: DRM plane
- * @extra_sources: Bitmask of additional supported pixel_sources for the
driver.
- *DRM_PLANE_PIXEL_SOURCE_FB and DRM_PLANE_PIXEL_SOURCE_NONE
will
- *always be enabled as supported sources.
- *
- * This creates a new property describing the current source of pixel data for
the
- * plane. The pixel_source will be initialized as DRM_PLANE_PIXEL_SOURCE_FB by
default.
- *
- * Drivers can set a custom default source by overriding the pixel_source
value in
- * drm_plane_funcs.reset()
- *
- * The property is exposed to userspace as an enumeration property called
- * "pixel_source" and has the following enumeration values:
- *
- * "NONE":
- * No active pixel source
- *
- * "FB":
- * Frameb
[PATCH 6/7] Revert "drm: Introduce solid fill DRM plane property"
This reverts commit 85863a4e16e77079ee14865905ddc3ef9483a640.
Altough the Solid Fill planes patchset got all reviews and
acknowledgements, it doesn't fulfill requirements for the new uABI. It
has neither corresponding open-source userspace implementation nor the
IGT tests coverage. Reverting this patchset until userspace obligations
are fulfilled.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_atomic_state_helper.c | 9 --
drivers/gpu/drm/drm_atomic_uapi.c | 26
drivers/gpu/drm/drm_blend.c | 30 ---
include/drm/drm_blend.h | 1 -
include/drm/drm_plane.h | 36 ---
include/uapi/drm/drm_mode.h | 24 ---
6 files changed, 126 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index f6c76cea8be4..311b04edf742 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -254,11 +254,6 @@ void __drm_atomic_helper_plane_state_reset(struct
drm_plane_state *plane_state,
plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
- if (plane_state->solid_fill_blob) {
- drm_property_blob_put(plane_state->solid_fill_blob);
- plane_state->solid_fill_blob = NULL;
- }
-
if (plane->color_encoding_property) {
if (!drm_object_property_get_default_value(&plane->base,
plane->color_encoding_property,
@@ -355,9 +350,6 @@ void __drm_atomic_helper_plane_duplicate_state(struct
drm_plane *plane,
if (state->fb)
drm_framebuffer_get(state->fb);
- if (state->solid_fill_blob)
- drm_property_blob_get(state->solid_fill_blob);
-
state->fence = NULL;
state->commit = NULL;
state->fb_damage_clips = NULL;
@@ -407,7 +399,6 @@ void __drm_atomic_helper_plane_destroy_state(struct
drm_plane_state *state)
drm_crtc_commit_put(state->commit);
drm_property_blob_put(state->fb_damage_clips);
- drm_property_blob_put(state->solid_fill_blob);
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index d7ae8e2c0265..bd7140531948 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -316,20 +316,6 @@ drm_atomic_set_crtc_for_connector(struct
drm_connector_state *conn_state,
}
EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
-static void drm_atomic_set_solid_fill_prop(struct drm_plane_state *state)
-{
- struct drm_mode_solid_fill *user_info;
-
- if (!state->solid_fill_blob)
- return;
-
- user_info = (struct drm_mode_solid_fill *)state->solid_fill_blob->data;
-
- state->solid_fill.r = user_info->r;
- state->solid_fill.g = user_info->g;
- state->solid_fill.b = user_info->b;
-}
-
static void set_out_fence_for_crtc(struct drm_atomic_state *state,
struct drm_crtc *crtc, s32 __user *fence_ptr)
{
@@ -578,15 +564,6 @@ static int drm_atomic_plane_set_property(struct drm_plane
*plane,
state->src_h = val;
} else if (property == plane->pixel_source_property) {
state->pixel_source = val;
- } else if (property == plane->solid_fill_property) {
- ret = drm_atomic_replace_property_blob_from_id(dev,
- &state->solid_fill_blob,
- val, sizeof(struct drm_mode_solid_fill),
- -1, &replaced);
- if (ret)
- return ret;
-
- drm_atomic_set_solid_fill_prop(state);
} else if (property == plane->alpha_property) {
state->alpha = val;
} else if (property == plane->blend_mode_property) {
@@ -677,9 +654,6 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_h;
} else if (property == plane->pixel_source_property) {
*val = state->pixel_source;
- } else if (property == plane->solid_fill_property) {
- *val = state->solid_fill_blob ?
- state->solid_fill_blob->base.id : 0;
} else if (property == plane->alpha_property) {
*val = state->alpha;
} else if (property == plane->blend_mode_property) {
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 665c5d9b056f..fce734cdb85b 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -204,10 +204,6 @@
* "FB":
* Framebuffer source set by the "FB_ID" property.
*
- * solid_fill:
- * solid_fill is set up with drm_plane_create_solid_fill_property(). It
- * cont
[PATCH 3/7] Revert "drm/atomic: Add solid fill data to plane state dump"
This reverts commit e86413f5442ee094e66b3e75f2d3419ed0df9520.
Altough the Solid Fill planes patchset got all reviews and
acknowledgements, it doesn't fulfill requirements for the new uABI. It
has neither corresponding open-source userspace implementation nor the
IGT tests coverage. Reverting this patchset until userspace obligations
are fulfilled.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_atomic.c | 4
drivers/gpu/drm/drm_plane.c | 8
include/drm/drm_plane.h | 3 ---
3 files changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1339785fbe80..02aa7df832cc 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -726,10 +726,6 @@ static void drm_atomic_plane_print_state(struct
drm_printer *p,
drm_printf(p, "\tfb=%u\n", state->fb ? state->fb->base.id : 0);
if (state->fb)
drm_framebuffer_print_info(p, 2, state->fb);
- drm_printf(p, "\tsolid_fill=%u\n",
- state->solid_fill_blob ?
state->solid_fill_blob->base.id : 0);
- if (state->solid_fill_blob)
- drm_plane_solid_fill_print_info(p, 2, state);
drm_printf(p, "\tcrtc-pos=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&dest));
drm_printf(p, "\tsrc-pos=" DRM_RECT_FP_FMT "\n", DRM_RECT_FP_ARG(&src));
drm_printf(p, "\trotation=%x\n", state->rotation);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index c65c72701dd2..c8dbe5ae60cc 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -1641,14 +1641,6 @@ __drm_plane_get_damage_clips(const struct
drm_plane_state *state)
state->fb_damage_clips->data : NULL);
}
-void drm_plane_solid_fill_print_info(struct drm_printer *p, unsigned int
indent,
-const struct drm_plane_state *state)
-{
- drm_printf_indent(p, indent, "r=0x%08x\n", state->solid_fill.r);
- drm_printf_indent(p, indent, "g=0x%08x\n", state->solid_fill.g);
- drm_printf_indent(p, indent, "b=0x%08x\n", state->solid_fill.b);
-}
-
/**
* drm_plane_get_damage_clips - Returns damage clips.
* @state: Plane state.
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index d14e2f1db234..4b7af4381bbe 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -1025,9 +1025,6 @@ drm_plane_get_damage_clips_count(const struct
drm_plane_state *state);
struct drm_mode_rect *
drm_plane_get_damage_clips(const struct drm_plane_state *state);
-void drm_plane_solid_fill_print_info(struct drm_printer *p, unsigned int
indent,
-const struct drm_plane_state *state);
-
int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
unsigned int supported_filters);
--
2.42.0
