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@xxxxxxxxxxxx>
on the mgag200 parts.
And
Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
on the drm_simple_kms_helper parts.
Sam
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
> 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
