Re: [PATCH 11/12] drm/mgag200: Introduce custom CRTC state

Hi Thomas,

On Mon, Jul 05, 2021 at 02:45:14PM +0200, Thomas Zimmermann wrote:

The CRTC state in mgag200 will hold PLL values for modeset operations. Simple KMS helpers already support custom state for planes. Extend the helpers to support custom CRTC state as well.

This should be split in two patches - so the changes to drm_simple_kms_helper can get more attention.

The patch as such looks fine so when split you can add my: Acked-by: Sam Ravnborg sam@ravnborg.org on the mgag200 parts.

And Reviewed-by: Sam Ravnborg sam@ravnborg.org on the drm_simple_kms_helper parts.

Sam

Signed-off-by: Thomas Zimmermann tzimmermann@suse.de

drivers/gpu/drm/drm_simple_kms_helper.c | 39 +++++++++++++++++++-- drivers/gpu/drm/mgag200/mgag200_drv.h | 9 +++++ drivers/gpu/drm/mgag200/mgag200_mode.c | 46 +++++++++++++++++++++++++ include/drm/drm_simple_kms_helper.h | 27 +++++++++++++++ 4 files changed, 118 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 735f4f34bcc4..72989ed1baba 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -145,6 +145,39 @@ static const struct drm_crtc_helper_funcs drm_simple_kms_crtc_helper_funcs = { .atomic_disable = drm_simple_kms_crtc_disable, };

+static void drm_simple_kms_crtc_reset(struct drm_crtc *crtc) +{

• struct drm_simple_display_pipe *pipe;
• pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
• if (!pipe->funcs || !pipe->funcs->reset_crtc)

• return drm_atomic_helper_crtc_reset(crtc);
  

• return pipe->funcs->reset_crtc(pipe);

+}

+static struct drm_crtc_state *drm_simple_kms_crtc_duplicate_state(struct drm_crtc *crtc) +{

• struct drm_simple_display_pipe *pipe;
• pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
• if (!pipe->funcs || !pipe->funcs->duplicate_crtc_state)

• return drm_atomic_helper_crtc_duplicate_state(crtc);
  

• return pipe->funcs->duplicate_crtc_state(pipe);

+}

+static void drm_simple_kms_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state) +{

• struct drm_simple_display_pipe *pipe;
• pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
• if (!pipe->funcs || !pipe->funcs->destroy_crtc_state)

• drm_atomic_helper_crtc_destroy_state(crtc, state);
  

• else

• pipe->funcs->destroy_crtc_state(pipe, state);
  

+}

static int drm_simple_kms_crtc_enable_vblank(struct drm_crtc *crtc) { struct drm_simple_display_pipe *pipe; @@ -168,12 +201,12 @@ static void drm_simple_kms_crtc_disable_vblank(struct drm_crtc *crtc) }

static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs = {

• .reset = drm_atomic_helper_crtc_reset,

• .reset = drm_simple_kms_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,

• .atomic_duplicate_state = drm_simple_kms_crtc_duplicate_state,
• .atomic_destroy_state = drm_simple_kms_crtc_destroy_state, .enable_vblank = drm_simple_kms_crtc_enable_vblank, .disable_vblank = drm_simple_kms_crtc_disable_vblank,

}; diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index fca3904fde0c..c813d6c15f70 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -127,6 +127,15 @@ struct mgag200_pll_values { unsigned int s; };

+struct mgag200_crtc_state {

• struct drm_crtc_state base;

+};

+static inline struct mgag200_crtc_state *to_mgag200_crtc_state(struct drm_crtc_state *base) +{

• return container_of(base, struct mgag200_crtc_state, base);

+}

#define to_mga_connector(x) container_of(x, struct mga_connector, base)

struct mga_i2c_chan { diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 5da72ebd8a7f..6a5c419c6641 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1886,6 +1886,49 @@ mgag200_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, mgag200_handle_damage(mdev, fb, &damage, &shadow_plane_state->map[0]); }

+static struct drm_crtc_state * +mgag200_simple_display_pipe_duplicate_crtc_state(struct drm_simple_display_pipe *pipe) +{

• struct drm_crtc *crtc = &pipe->crtc;
• struct drm_crtc_state *crtc_state = crtc->state;
• struct mgag200_crtc_state *new_mgag200_crtc_state;
• if (!crtc_state)

• return NULL;
  

• new_mgag200_crtc_state = kzalloc(sizeof(*new_mgag200_crtc_state), GFP_KERNEL);
• if (!new_mgag200_crtc_state)

• return NULL;
  

• __drm_atomic_helper_crtc_duplicate_state(crtc, &new_mgag200_crtc_state->base);
• return &new_mgag200_crtc_state->base;

+}

+static void mgag200_simple_display_pipe_destroy_crtc_state(struct drm_simple_display_pipe *pipe,

• 					   struct drm_crtc_state *crtc_state)
  

+{

• struct mgag200_crtc_state *mgag200_crtc_state = to_mgag200_crtc_state(crtc_state);
• __drm_atomic_helper_crtc_destroy_state(&mgag200_crtc_state->base);
• kfree(mgag200_crtc_state);

+}

+static void mgag200_simple_display_pipe_reset_crtc(struct drm_simple_display_pipe *pipe) +{

• struct drm_crtc *crtc = &pipe->crtc;
• struct mgag200_crtc_state *mgag200_crtc_state;
• if (crtc->state) {

• mgag200_simple_display_pipe_destroy_crtc_state(pipe, crtc->state);
  

• crtc->state = NULL; /* must be set to NULL here */
  

• }
• mgag200_crtc_state = kzalloc(sizeof(*mgag200_crtc_state), GFP_KERNEL);
• if (!mgag200_crtc_state)

• return;
  

• __drm_atomic_helper_crtc_reset(crtc, &mgag200_crtc_state->base);

+}

static const struct drm_simple_display_pipe_funcs mgag200_simple_display_pipe_funcs = { .mode_valid = mgag200_simple_display_pipe_mode_valid, @@ -1893,6 +1936,9 @@ mgag200_simple_display_pipe_funcs = { .disable = mgag200_simple_display_pipe_disable, .check = mgag200_simple_display_pipe_check, .update = mgag200_simple_display_pipe_update,

• .reset_crtc = mgag200_simple_display_pipe_reset_crtc,
• .duplicate_crtc_state = mgag200_simple_display_pipe_duplicate_crtc_state,
• .destroy_crtc_state = mgag200_simple_display_pipe_destroy_crtc_state, DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS,

};

diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h index cf07132d4ee8..0b3647e614dd 100644 --- a/include/drm/drm_simple_kms_helper.h +++ b/include/drm/drm_simple_kms_helper.h @@ -153,6 +153,33 @@ struct drm_simple_display_pipe_funcs { */ void (*disable_vblank)(struct drm_simple_display_pipe *pipe);

• /**

• * @reset_crtc:
  

• *
  

• * Optional, called by &drm_crtc_funcs.reset. Please read the
  

• * documentation for the &drm_crtc_funcs.reset hook for more details.
  

• */
  

• void (*reset_crtc)(struct drm_simple_display_pipe *pipe);
• /**

• * @duplicate_crtc_state:
  

• *
  

• * Optional, called by &drm_crtc_funcs.atomic_duplicate_state. Please
  

• * read the documentation for the &drm_crtc_funcs.atomic_duplicate_state
  

• * hook for more details.
  

• */
  

• struct drm_crtc_state * (*duplicate_crtc_state)(struct drm_simple_display_pipe *pipe);
• /**

• * @destroy_crtc_state:
  

• *
  

• * Optional, called by &drm_crtc_funcs.atomic_destroy_state. Please
  

• * read the documentation for the &drm_crtc_funcs.atomic_destroy_state
  

• * hook for more details.
  

• */
  

• void (*destroy_crtc_state)(struct drm_simple_display_pipe *pipe,

• 		   struct drm_crtc_state *crtc_state);
  

• /**
  • @reset_plane:

-- 2.32.0