| Message ID | 20180319173113.94879-1-seanpaul@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2018-03-19 10:31, Sean Paul wrote: > Instead of subclassing atomic state, store driver private data in > private_obj/state. This allows us to remove the swap_state driver hook > for mdp5 and get closer to using the atomic helpers entirely. > > Changes in v2: > - Use state->state in disp duplicate_state callback (Jeykumar) > > Cc: Jeykumar Sankaran <jsanka@codeaurora.org> > Signed-off-by: Sean Paul <seanpaul@chromium.org> With the below comment updated Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org> > --- > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 37 ++++++-------- > drivers/gpu/drm/msm/msm_atomic.c | 30 ----------- > drivers/gpu/drm/msm/msm_drv.c | 65 ++++++++++++++++++++++-- > drivers/gpu/drm/msm/msm_drv.h | 4 +- > drivers/gpu/drm/msm/msm_kms.h | 28 +++++----- > 5 files changed, 95 insertions(+), 69 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > index 6d8e3a9a6fc0..f1a248419655 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > @@ -74,36 +74,32 @@ struct mdp5_state *mdp5_get_state(struct > drm_atomic_state *s) > { > struct msm_drm_private *priv = s->dev->dev_private; > struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms)); > - struct msm_kms_state *state = to_kms_state(s); > - struct mdp5_state *new_state; > + struct msm_kms_state *kms_state; > int ret; > > - if (state->state) > - return state->state; > - > ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx); > if (ret) > return ERR_PTR(ret); > > - new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL); > - if (!new_state) > - return ERR_PTR(-ENOMEM); > + kms_state = msm_kms_get_state(s); > + if (IS_ERR_OR_NULL(kms_state)) > + return (struct mdp5_state *)kms_state; /* casting ERR_PTR > */ > > - /* Copy state: */ > - new_state->hwpipe = mdp5_kms->state->hwpipe; > - new_state->hwmixer = mdp5_kms->state->hwmixer; > - if (mdp5_kms->smp) > - new_state->smp = mdp5_kms->state->smp; > + return kms_state->state; > +} > > - state->state = new_state; > +static void *mdp5_duplicate_state(void *state) > +{ > + if (!state) > + return kzalloc(sizeof(struct mdp5_state), GFP_KERNEL); > > - return new_state; > + return kmemdup(state, sizeof(struct mdp5_state), GFP_KERNEL); > } > > -static void mdp5_swap_state(struct msm_kms *kms, struct > drm_atomic_state > *state) > +static void mdp5_destroy_state(void *state) > { > - struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); > - swap(to_kms_state(state)->state, mdp5_kms->state); > + struct mdp5_state *mdp_state = state; > + kfree(mdp_state); > } > > static void mdp5_prepare_commit(struct msm_kms *kms, struct > drm_atomic_state *state) > @@ -229,7 +225,8 @@ static const struct mdp_kms_funcs kms_funcs = { > .irq = mdp5_irq, > .enable_vblank = mdp5_enable_vblank, > .disable_vblank = mdp5_disable_vblank, > - .swap_state = mdp5_swap_state, > + .duplicate_state = mdp5_duplicate_state, > + .destroy_state = mdp5_destroy_state, > .prepare_commit = mdp5_prepare_commit, > .complete_commit = mdp5_complete_commit, > .wait_for_crtc_commit_done = > mdp5_wait_for_crtc_commit_done, > @@ -726,8 +723,6 @@ static void mdp5_destroy(struct platform_device > *pdev) > > if (mdp5_kms->rpm_enabled) > pm_runtime_disable(&pdev->dev); > - > - kfree(mdp5_kms->state); > } > > static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt, > diff --git a/drivers/gpu/drm/msm/msm_atomic.c > b/drivers/gpu/drm/msm/msm_atomic.c > index 7e54eb65d096..1f53262ea46b 100644 > --- a/drivers/gpu/drm/msm/msm_atomic.c > +++ b/drivers/gpu/drm/msm/msm_atomic.c > @@ -169,9 +169,6 @@ int msm_atomic_commit(struct drm_device *dev, > */ > BUG_ON(drm_atomic_helper_swap_state(state, false) < 0); > > - if (to_kms_state(state)->state) > - priv->kms->funcs->swap_state(priv->kms, state); > - > /* > * Provide the driver a chance to prepare for output fences. This > is > * done after the point of no return, but before asynchronous > commits > @@ -210,30 +207,3 @@ int msm_atomic_commit(struct drm_device *dev, > drm_atomic_helper_cleanup_planes(dev, state); > return ret; > } > - > -struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device > *dev) > -{ > - struct msm_kms_state *state = kzalloc(sizeof(*state), GFP_KERNEL); > - > - if (!state || drm_atomic_state_init(dev, &state->base) < 0) { > - kfree(state); > - return NULL; > - } > - > - return &state->base; > -} > - > -void msm_atomic_state_clear(struct drm_atomic_state *s) > -{ > - struct msm_kms_state *state = to_kms_state(s); > - drm_atomic_state_default_clear(&state->base); > - kfree(state->state); > - state->state = NULL; > -} > - > -void msm_atomic_state_free(struct drm_atomic_state *state) > -{ > - kfree(to_kms_state(state)->state); > - drm_atomic_state_default_release(state); > - kfree(state); > -} > diff --git a/drivers/gpu/drm/msm/msm_drv.c > b/drivers/gpu/drm/msm/msm_drv.c > index 5219792e93bc..4c0740dc7854 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c > @@ -121,9 +121,62 @@ static const struct drm_mode_config_funcs > mode_config_funcs = { > .output_poll_changed = msm_fb_output_poll_changed, > .atomic_check = drm_atomic_helper_check, > .atomic_commit = msm_atomic_commit, > - .atomic_state_alloc = msm_atomic_state_alloc, > - .atomic_state_clear = msm_atomic_state_clear, > - .atomic_state_free = msm_atomic_state_free, > +}; > + > +static inline > +struct msm_kms *obj_to_kms(struct drm_private_obj *obj) > +{ > + return container_of(obj, struct msm_kms, base); > +} > + > +static inline > +struct msm_kms_state *state_to_kms_state(struct drm_private_state > *state) > +{ > + return container_of(state, struct msm_kms_state, base); > +} > + > +struct msm_kms_state *msm_kms_get_state(struct drm_atomic_state *s) > +{ > + struct msm_drm_private *priv = s->dev->dev_private; > + struct drm_private_obj *obj = &priv->kms->base; > + struct drm_private_state *state; > + > + state = drm_atomic_get_private_obj_state(s, obj); > + if (IS_ERR_OR_NULL(state)) > + return (struct msm_kms_state *)state; /* casting ERR_PTR > */ > + > + return state_to_kms_state(state); > +} > + > +static struct drm_private_state * > +msm_kms_duplicate_state(struct drm_private_obj *obj) > +{ > + struct msm_kms *kms = obj_to_kms(obj); > + struct msm_kms_state *state = state_to_kms_state(obj->state); > + struct msm_kms_state *new_state; > + > + if (kms->funcs->duplicate_state) > + new_state = kms->funcs->duplicate_state(state->state); > + > + __drm_atomic_helper_private_obj_duplicate_state(obj, > &new_state->base); > + > + return &new_state->base; > +} > + > +static void msm_kms_destroy_state(struct drm_private_obj *obj, > + struct drm_private_state *state) > +{ > + struct msm_kms *kms = obj_to_kms(obj); > + struct msm_kms_state *kms_state = state_to_kms_state(obj->state); > + > + if (kms->funcs->destroy_state) > + kms->funcs->destroy_state(kms_state->state); > + kfree(kms_state); > +} > + > +static const struct drm_private_state_funcs kms_state_funcs = { > + .atomic_duplicate_state = msm_kms_duplicate_state, > + .atomic_destroy_state = msm_kms_destroy_state, > }; > > #ifdef CONFIG_DRM_MSM_REGISTER_LOGGING > @@ -341,6 +394,8 @@ static int msm_drm_uninit(struct device *dev) > } > } > > + drm_atomic_private_obj_fini(&kms->base); > + > msm_gem_shrinker_cleanup(ddev); > > drm_kms_helper_poll_fini(ddev); > @@ -770,6 +825,10 @@ static int msm_drm_init(struct device *dev, struct > drm_driver *drv) > } > #endif > > + drm_atomic_private_obj_init(&kms->base, > + &kms->state.base, > + &kms_state_funcs); > + > /* perform subdriver post initialization */ > if (kms && kms->funcs && kms->funcs->postinit) { > ret = kms->funcs->postinit(kms); > diff --git a/drivers/gpu/drm/msm/msm_drv.h > b/drivers/gpu/drm/msm/msm_drv.h > index 292496b682e8..8cab333df717 100644 > --- a/drivers/gpu/drm/msm/msm_drv.h > +++ b/drivers/gpu/drm/msm/msm_drv.h > @@ -598,9 +598,7 @@ struct msm_format { > > int msm_atomic_commit(struct drm_device *dev, > struct drm_atomic_state *state, bool nonblock); > -struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device > *dev); > -void msm_atomic_state_clear(struct drm_atomic_state *state); > -void msm_atomic_state_free(struct drm_atomic_state *state); > +struct msm_kms_state *msm_kms_get_state(struct drm_atomic_state > *state); > > void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, > struct msm_gem_vma *vma, struct sg_table *sgt); > diff --git a/drivers/gpu/drm/msm/msm_kms.h > b/drivers/gpu/drm/msm/msm_kms.h > index 09ba1e79db50..4c5a69258c42 100644 > --- a/drivers/gpu/drm/msm/msm_kms.h > +++ b/drivers/gpu/drm/msm/msm_kms.h > @@ -57,6 +57,8 @@ struct msm_kms_funcs { > void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc > *crtc); > /* swap global atomic state: */ > void (*swap_state)(struct msm_kms *kms, struct drm_atomic_state > *state); > + void *(*duplicate_state)(void *state); > + void (*destroy_state)(void *state); > /* modeset, bracketing atomic_commit(): */ > void (*prepare_fence)(struct msm_kms *kms, > struct drm_atomic_state *state); > @@ -114,16 +116,6 @@ struct msm_kms_funcs { > #endif > }; > > -struct msm_kms { > - const struct msm_kms_funcs *funcs; > - > - /* irq number to be passed on to drm_irq_install */ > - int irq; > - > - /* mapper-id used to request GEM buffer mapped for scanout: */ > - struct msm_gem_address_space *aspace; > -}; > - > /** > * Subclass of drm_atomic_state, to allow kms backend to have driver > * private global state. The kms backend can do whatever it wants Update comments. It is not subclassing drm_atomic_state anymore. > @@ -131,10 +123,22 @@ struct msm_kms { > * is kfree'd and set back to NULL. > */ > struct msm_kms_state { > - struct drm_atomic_state base; > + struct drm_private_state base; > void *state; > }; > -#define to_kms_state(x) container_of(x, struct msm_kms_state, base) > + > +struct msm_kms { > + struct drm_private_obj base; > + struct msm_kms_state state; > + > + const struct msm_kms_funcs *funcs; > + > + /* irq number to be passed on to drm_irq_install */ > + int irq; > + > + /* mapper-id used to request GEM buffer mapped for scanout: */ > + struct msm_gem_address_space *aspace; > +}; > > static inline void msm_kms_init(struct msm_kms *kms, > const struct msm_kms_funcs *funcs)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 6d8e3a9a6fc0..f1a248419655 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -74,36 +74,32 @@ struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s) { struct msm_drm_private *priv = s->dev->dev_private; struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms)); - struct msm_kms_state *state = to_kms_state(s); - struct mdp5_state *new_state; + struct msm_kms_state *kms_state; int ret; - if (state->state) - return state->state; - ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx); if (ret) return ERR_PTR(ret); - new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL); - if (!new_state) - return ERR_PTR(-ENOMEM); + kms_state = msm_kms_get_state(s); + if (IS_ERR_OR_NULL(kms_state)) + return (struct mdp5_state *)kms_state; /* casting ERR_PTR */ - /* Copy state: */ - new_state->hwpipe = mdp5_kms->state->hwpipe; - new_state->hwmixer = mdp5_kms->state->hwmixer; - if (mdp5_kms->smp) - new_state->smp = mdp5_kms->state->smp; + return kms_state->state; +} - state->state = new_state; +static void *mdp5_duplicate_state(void *state) +{ + if (!state) + return kzalloc(sizeof(struct mdp5_state), GFP_KERNEL); - return new_state; + return kmemdup(state, sizeof(struct mdp5_state), GFP_KERNEL); } -static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state) +static void mdp5_destroy_state(void *state) { - struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); - swap(to_kms_state(state)->state, mdp5_kms->state); + struct mdp5_state *mdp_state = state; + kfree(mdp_state); } static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state) @@ -229,7 +225,8 @@ static const struct mdp_kms_funcs kms_funcs = { .irq = mdp5_irq, .enable_vblank = mdp5_enable_vblank, .disable_vblank = mdp5_disable_vblank, - .swap_state = mdp5_swap_state, + .duplicate_state = mdp5_duplicate_state, + .destroy_state = mdp5_destroy_state, .prepare_commit = mdp5_prepare_commit, .complete_commit = mdp5_complete_commit, .wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done, @@ -726,8 +723,6 @@ static void mdp5_destroy(struct platform_device *pdev) if (mdp5_kms->rpm_enabled) pm_runtime_disable(&pdev->dev); - - kfree(mdp5_kms->state); } static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt, diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 7e54eb65d096..1f53262ea46b 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -169,9 +169,6 @@ int msm_atomic_commit(struct drm_device *dev, */ BUG_ON(drm_atomic_helper_swap_state(state, false) < 0); - if (to_kms_state(state)->state) - priv->kms->funcs->swap_state(priv->kms, state); - /* * Provide the driver a chance to prepare for output fences. This is * done after the point of no return, but before asynchronous commits @@ -210,30 +207,3 @@ int msm_atomic_commit(struct drm_device *dev, drm_atomic_helper_cleanup_planes(dev, state); return ret; } - -struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev) -{ - struct msm_kms_state *state = kzalloc(sizeof(*state), GFP_KERNEL); - - if (!state || drm_atomic_state_init(dev, &state->base) < 0) { - kfree(state); - return NULL; - } - - return &state->base; -} - -void msm_atomic_state_clear(struct drm_atomic_state *s) -{ - struct msm_kms_state *state = to_kms_state(s); - drm_atomic_state_default_clear(&state->base); - kfree(state->state); - state->state = NULL; -} - -void msm_atomic_state_free(struct drm_atomic_state *state) -{ - kfree(to_kms_state(state)->state); - drm_atomic_state_default_release(state); - kfree(state); -} diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 5219792e93bc..4c0740dc7854 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -121,9 +121,62 @@ static const struct drm_mode_config_funcs mode_config_funcs = { .output_poll_changed = msm_fb_output_poll_changed, .atomic_check = drm_atomic_helper_check, .atomic_commit = msm_atomic_commit, - .atomic_state_alloc = msm_atomic_state_alloc, - .atomic_state_clear = msm_atomic_state_clear, - .atomic_state_free = msm_atomic_state_free, +}; + +static inline +struct msm_kms *obj_to_kms(struct drm_private_obj *obj) +{ + return container_of(obj, struct msm_kms, base); +} + +static inline +struct msm_kms_state *state_to_kms_state(struct drm_private_state *state) +{ + return container_of(state, struct msm_kms_state, base); +} + +struct msm_kms_state *msm_kms_get_state(struct drm_atomic_state *s) +{ + struct msm_drm_private *priv = s->dev->dev_private; + struct drm_private_obj *obj = &priv->kms->base; + struct drm_private_state *state; + + state = drm_atomic_get_private_obj_state(s, obj); + if (IS_ERR_OR_NULL(state)) + return (struct msm_kms_state *)state; /* casting ERR_PTR */ + + return state_to_kms_state(state); +} + +static struct drm_private_state * +msm_kms_duplicate_state(struct drm_private_obj *obj) +{ + struct msm_kms *kms = obj_to_kms(obj); + struct msm_kms_state *state = state_to_kms_state(obj->state); + struct msm_kms_state *new_state; + + if (kms->funcs->duplicate_state) + new_state = kms->funcs->duplicate_state(state->state); + + __drm_atomic_helper_private_obj_duplicate_state(obj, &new_state->base); + + return &new_state->base; +} + +static void msm_kms_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + struct msm_kms *kms = obj_to_kms(obj); + struct msm_kms_state *kms_state = state_to_kms_state(obj->state); + + if (kms->funcs->destroy_state) + kms->funcs->destroy_state(kms_state->state); + kfree(kms_state); +} + +static const struct drm_private_state_funcs kms_state_funcs = { + .atomic_duplicate_state = msm_kms_duplicate_state, + .atomic_destroy_state = msm_kms_destroy_state, }; #ifdef CONFIG_DRM_MSM_REGISTER_LOGGING @@ -341,6 +394,8 @@ static int msm_drm_uninit(struct device *dev) } } + drm_atomic_private_obj_fini(&kms->base); + msm_gem_shrinker_cleanup(ddev); drm_kms_helper_poll_fini(ddev); @@ -770,6 +825,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) } #endif + drm_atomic_private_obj_init(&kms->base, + &kms->state.base, + &kms_state_funcs); + /* perform subdriver post initialization */ if (kms && kms->funcs && kms->funcs->postinit) { ret = kms->funcs->postinit(kms); diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 292496b682e8..8cab333df717 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -598,9 +598,7 @@ struct msm_format { int msm_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state, bool nonblock); -struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev); -void msm_atomic_state_clear(struct drm_atomic_state *state); -void msm_atomic_state_free(struct drm_atomic_state *state); +struct msm_kms_state *msm_kms_get_state(struct drm_atomic_state *state); void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, struct sg_table *sgt); diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index 09ba1e79db50..4c5a69258c42 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -57,6 +57,8 @@ struct msm_kms_funcs { void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc); /* swap global atomic state: */ void (*swap_state)(struct msm_kms *kms, struct drm_atomic_state *state); + void *(*duplicate_state)(void *state); + void (*destroy_state)(void *state); /* modeset, bracketing atomic_commit(): */ void (*prepare_fence)(struct msm_kms *kms, struct drm_atomic_state *state); @@ -114,16 +116,6 @@ struct msm_kms_funcs { #endif }; -struct msm_kms { - const struct msm_kms_funcs *funcs; - - /* irq number to be passed on to drm_irq_install */ - int irq; - - /* mapper-id used to request GEM buffer mapped for scanout: */ - struct msm_gem_address_space *aspace; -}; - /** * Subclass of drm_atomic_state, to allow kms backend to have driver * private global state. The kms backend can do whatever it wants @@ -131,10 +123,22 @@ struct msm_kms { * is kfree'd and set back to NULL. */ struct msm_kms_state { - struct drm_atomic_state base; + struct drm_private_state base; void *state; }; -#define to_kms_state(x) container_of(x, struct msm_kms_state, base) + +struct msm_kms { + struct drm_private_obj base; + struct msm_kms_state state; + + const struct msm_kms_funcs *funcs; + + /* irq number to be passed on to drm_irq_install */ + int irq; + + /* mapper-id used to request GEM buffer mapped for scanout: */ + struct msm_gem_address_space *aspace; +}; static inline void msm_kms_init(struct msm_kms *kms, const struct msm_kms_funcs *funcs)
Instead of subclassing atomic state, store driver private data in private_obj/state. This allows us to remove the swap_state driver hook for mdp5 and get closer to using the atomic helpers entirely. Changes in v2: - Use state->state in disp duplicate_state callback (Jeykumar) Cc: Jeykumar Sankaran <jsanka@codeaurora.org> Signed-off-by: Sean Paul <seanpaul@chromium.org> --- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 37 ++++++-------- drivers/gpu/drm/msm/msm_atomic.c | 30 ----------- drivers/gpu/drm/msm/msm_drv.c | 65 ++++++++++++++++++++++-- drivers/gpu/drm/msm/msm_drv.h | 4 +- drivers/gpu/drm/msm/msm_kms.h | 28 +++++----- 5 files changed, 95 insertions(+), 69 deletions(-)