Hi
Am 20.04.25 um 14:19 schrieb Ryosuke Yasuoka:
Drop simple-KMS in favor of regular atomic helpers to make the code more
modular. The simple-KMS helper mix up plane and CRTC state, so it is
obsolete and should go away [1]. Since it just split the simple-pipe
funtions into per-plane and per-CRTC, no functional changes is expected.
[1] https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Ryosuke Yasuoka <[email protected]>
I cannot test, but thanks for doing this. Comments below.
---
drivers/gpu/drm/hyperv/hyperv_drm.h | 4 +-
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 168 ++++++++++++++++----
2 files changed, 139 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm.h
b/drivers/gpu/drm/hyperv/hyperv_drm.h
index d2d8582b36df..9e776112c03e 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm.h
+++ b/drivers/gpu/drm/hyperv/hyperv_drm.h
@@ -11,7 +11,9 @@
struct hyperv_drm_device {
/* drm */
struct drm_device dev;
- struct drm_simple_display_pipe pipe;
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
struct drm_connector connector;
/* mode */
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
index 6c6b57298797..c273c093b491 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
@@ -5,6 +5,7 @@
#include <linux/hyperv.h>
+#include <drm/drm_atomic.h>
#include <drm/drm_damage_helper.h>
#include <drm/drm_drv.h>
#include <drm/drm_edid.h>
@@ -15,7 +16,7 @@
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_gem_shmem_helper.h>
#include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_plane.h>
#include "hyperv_drm.h"
@@ -98,12 +99,47 @@ static int hyperv_check_size(struct hyperv_drm_device *hv, int w, int h,
return 0;
}
-static void hyperv_pipe_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+static const uint32_t hyperv_formats[] = {
+ DRM_FORMAT_XRGB8888,
+};
+
+static const uint64_t hyperv_modifiers[] = {
+ DRM_FORMAT_MOD_LINEAR,
+ DRM_FORMAT_MOD_INVALID
+};
+
+static enum drm_mode_status
+hyperv_crtc_helper_mode_valid(struct drm_crtc *crtc,
+ const struct drm_display_mode *mode)
+{
+ return MODE_OK;
+}
This helper should not be necessary.
+
+static int hyperv_crtc_helper_atomic_check(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct hyperv_drm_device *hv = to_hv(pipe->crtc.dev);
+ struct drm_crtc_state *crtc_state =
drm_atomic_get_new_crtc_state(state, crtc);
+ int ret;
+
+ if (!crtc_state->enable)
+ goto out;
+
+ ret = drm_atomic_helper_check_crtc_primary_plane(crtc_state);
+ if (ret)
+ return ret;
+
+out:
+ return drm_atomic_add_affected_planes(state, crtc);
IIRC this function call was only required to work around an issue in the
simple-kms helpers. It's not necessary if you use the regular interface
and should be removed.
+}
+
+static void hyperv_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct hyperv_drm_device *hv = to_hv(crtc->dev);
+ struct drm_plane *plane = &hv->plane;
+ struct drm_plane_state *plane_state = plane->state;
struct drm_shadow_plane_state *shadow_plane_state =
to_drm_shadow_plane_state(plane_state);
+ struct drm_crtc_state *crtc_state = crtc->state;
hyperv_hide_hw_ptr(hv->hdev);
hyperv_update_situation(hv->hdev, 1, hv->screen_depth,
This looks more like something to do in the plane's atomic_enable, but
it's OK for now.
@@ -113,12 +149,48 @@ static void hyperv_pipe_enable(struct
drm_simple_display_pipe *pipe,
hyperv_blit_to_vram_fullscreen(plane_state->fb,
&shadow_plane_state->data[0]);
I think this can be removed. Blit'ing is for planes and the plane
already does it it's atomic_update. Doing this here as well is another
artifact of the simple-kms helpers.
}
-static int hyperv_pipe_check(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *plane_state,
- struct drm_crtc_state *crtc_state)
+static void hyperv_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{ }
+
+static const struct drm_crtc_helper_funcs hyperv_crtc_helper_funcs = {
+ .mode_valid = hyperv_crtc_helper_mode_valid,
+ .atomic_check = hyperv_crtc_helper_atomic_check,
+ .atomic_enable = hyperv_crtc_helper_atomic_enable,
+ .atomic_disable = hyperv_crtc_helper_atomic_disable,
+};
+
+static const struct drm_crtc_funcs hyperv_crtc_funcs = {
+ .reset = drm_atomic_helper_crtc_reset,
+ .destroy = drm_crtc_cleanup,
+ .set_config = drm_atomic_helper_set_config,
+ .page_flip = drm_atomic_helper_page_flip,
+ .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+};
+
+static int hyperv_plane_atomic_check(struct drm_plane *plane,
+ struct drm_atomic_state *state)
{
- struct hyperv_drm_device *hv = to_hv(pipe->crtc.dev);
+ struct drm_plane_state *plane_state =
drm_atomic_get_new_plane_state(state, plane);
+ struct hyperv_drm_device *hv = to_hv(plane->dev);
struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_crtc *crtc = plane_state->crtc;
+ struct drm_crtc_state *crtc_state = NULL;
+ int ret;
+
+ if (crtc)
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+
+ ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+ DRM_PLANE_NO_SCALING,
+ DRM_PLANE_NO_SCALING,
+ false, false);
+ if (ret)
+ return ret;
+
+ if (!plane_state->visible)
+ return 0;
if (fb->format->format != DRM_FORMAT_XRGB8888)
return -EINVAL;
Our DRM core should have done this test already.
@@ -132,51 +204,83 @@ static int hyperv_pipe_check(struct
drm_simple_display_pipe *pipe,
return 0;
}
-static void hyperv_pipe_update(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *old_state)
+static void hyperv_plane_atomic_update(struct drm_plane *plane,
+ struct drm_atomic_state
*old_state)
{
- struct hyperv_drm_device *hv = to_hv(pipe->crtc.dev);
- struct drm_plane_state *state = pipe->plane.state;
+ struct drm_plane_state *old_pstate =
drm_atomic_get_old_plane_state(old_state, plane);
+ struct hyperv_drm_device *hv = to_hv(plane->dev);
+ struct drm_plane_state *state = plane->state;
struct drm_shadow_plane_state *shadow_plane_state =
to_drm_shadow_plane_state(state);
struct drm_rect rect;
- if (drm_atomic_helper_damage_merged(old_state, state, &rect)) {
+ if (drm_atomic_helper_damage_merged(old_pstate, state, &rect)) {
hyperv_blit_to_vram_rect(state->fb, &shadow_plane_state->data[0],
&rect);
hyperv_update_dirt(hv->hdev, &rect);
}
}
-static const struct drm_simple_display_pipe_funcs hyperv_pipe_funcs = {
- .enable = hyperv_pipe_enable,
- .check = hyperv_pipe_check,
- .update = hyperv_pipe_update,
- DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS,
+static bool hyperv_format_mod_supported(struct drm_plane *plane,
+ uint32_t format, uint64_t modifier)
+{
+ return modifier == DRM_FORMAT_MOD_LINEAR;
+}
This helper is probably not necessary either.
+
+static const struct drm_plane_helper_funcs hyperv_plane_helper_funcs = {
+ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+ .atomic_check = hyperv_plane_atomic_check,
+ .atomic_update = hyperv_plane_atomic_update,
};
-static const uint32_t hyperv_formats[] = {
- DRM_FORMAT_XRGB8888,
+static const struct drm_plane_funcs hyperv_plane_funcs = {
+ .update_plane = drm_atomic_helper_update_plane,
+ .disable_plane = drm_atomic_helper_disable_plane,
+ .destroy = drm_plane_cleanup,
+ .format_mod_supported = hyperv_format_mod_supported,
+ DRM_GEM_SHADOW_PLANE_FUNCS,
};
-static const uint64_t hyperv_modifiers[] = {
- DRM_FORMAT_MOD_LINEAR,
- DRM_FORMAT_MOD_INVALID
+static const struct drm_encoder_funcs hyperv_drm_simple_encoder_funcs_cleanup
= {
+ .destroy = drm_encoder_cleanup,
};
static inline int hyperv_pipe_init(struct hyperv_drm_device *hv)
{
+ struct drm_device *dev = &hv->dev;
+ struct drm_encoder *encoder = &hv->encoder;
+ struct drm_plane *plane = &hv->plane;
+ struct drm_crtc *crtc = &hv->crtc;
+ struct drm_connector *connector = &hv->connector;
int ret;
- ret = drm_simple_display_pipe_init(&hv->dev,
- &hv->pipe,
- &hyperv_pipe_funcs,
- hyperv_formats,
- ARRAY_SIZE(hyperv_formats),
- hyperv_modifiers,
- &hv->connector);
+ drm_plane_helper_add(plane, &hyperv_plane_helper_funcs);
Rather call drm_universal_plane_init() first and then add the helper funcs.
+ ret = drm_universal_plane_init(dev, plane, 0,
+ &hyperv_plane_funcs,
+ hyperv_formats,
ARRAY_SIZE(hyperv_formats),
+ hyperv_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+
+ drm_crtc_helper_add(crtc, &hyperv_crtc_helper_funcs);
+ ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,
+ &hyperv_crtc_funcs, NULL);
Same ordering nit as with the plane.
+ if (ret)
+ return ret;
+
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+ ret = drm_encoder_init(dev, encoder,
+ &hyperv_drm_simple_encoder_funcs_cleanup,
+ DRM_MODE_ENCODER_NONE, NULL);
+
+ if (ret || !connector)
+ return ret;
Connector init happens elsewhere, but you can inline hyperv_conn_init()
right here.
+
+ ret = drm_connector_attach_encoder(connector, encoder);
+
if (ret)
return ret;
- drm_plane_enable_fb_damage_clips(&hv->pipe.plane);
+ drm_plane_enable_fb_damage_clips(&hv->plane);
This is a plane function you can move it close to the plane init.
Best regards
Thomas
return 0;
}
base-commit: b60301774a8fe6c30b14a95104ec099290a2e904
--
--
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)
