This series replaces the driver customized atomic_commit implementation with the drm core helpers (patch 1). The vblank event management is reworked (patch 2) and a side-effect is fixed (patch 3). Fabien
Since nonblocking atomic commits are now supported, the driver can now use drm_atomic_helper_commit().
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com --- drivers/gpu/drm/sti/sti_drv.c | 83 +------------------------------------------ drivers/gpu/drm/sti/sti_drv.h | 6 ---- 2 files changed, 1 insertion(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index ff71e25..9a9b9a2 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -150,52 +150,6 @@ static void sti_drm_dbg_cleanup(struct drm_minor *minor) 1, minor); }
-static void sti_atomic_schedule(struct sti_private *private, - struct drm_atomic_state *state) -{ - private->commit.state = state; - schedule_work(&private->commit.work); -} - -static void sti_atomic_complete(struct sti_private *private, - struct drm_atomic_state *state) -{ - struct drm_device *drm = private->drm_dev; - - /* - * Everything below can be run asynchronously without the need to grab - * any modeset locks at all under one condition: It must be guaranteed - * that the asynchronous work has either been cancelled (if the driver - * supports it, which at least requires that the framebuffers get - * cleaned up with drm_atomic_helper_cleanup_planes()) or completed - * before the new state gets committed on the software side with - * drm_atomic_helper_swap_state(). - * - * This scheme allows new atomic state updates to be prepared and - * checked in parallel to the asynchronous completion of the previous - * update. Which is important since compositors need to figure out the - * composition of the next frame right after having submitted the - * current layout. - */ - - drm_atomic_helper_commit_modeset_disables(drm, state); - drm_atomic_helper_commit_planes(drm, state, 0); - drm_atomic_helper_commit_modeset_enables(drm, state); - - drm_atomic_helper_wait_for_vblanks(drm, state); - - drm_atomic_helper_cleanup_planes(drm, state); - drm_atomic_state_put(state); -} - -static void sti_atomic_work(struct work_struct *work) -{ - struct sti_private *private = container_of(work, - struct sti_private, commit.work); - - sti_atomic_complete(private, private->commit.state); -} - static int sti_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) { @@ -216,38 +170,6 @@ static int sti_atomic_check(struct drm_device *dev, return ret; }
-static int sti_atomic_commit(struct drm_device *drm, - struct drm_atomic_state *state, bool nonblock) -{ - struct sti_private *private = drm->dev_private; - int err; - - err = drm_atomic_helper_prepare_planes(drm, state); - if (err) - return err; - - /* serialize outstanding nonblocking commits */ - mutex_lock(&private->commit.lock); - flush_work(&private->commit.work); - - /* - * This is the point of no return - everything below never fails except - * when the hw goes bonghits. Which means we can commit the new state on - * the software side now. - */ - - drm_atomic_helper_swap_state(state, true); - - drm_atomic_state_get(state); - if (nonblock) - sti_atomic_schedule(private, state); - else - sti_atomic_complete(private, state); - - mutex_unlock(&private->commit.lock); - return 0; -} - static void sti_output_poll_changed(struct drm_device *ddev) { struct sti_private *private = ddev->dev_private; @@ -271,7 +193,7 @@ static const struct drm_mode_config_funcs sti_mode_config_funcs = { .fb_create = drm_fb_cma_create, .output_poll_changed = sti_output_poll_changed, .atomic_check = sti_atomic_check, - .atomic_commit = sti_atomic_commit, + .atomic_commit = drm_atomic_helper_commit, };
static void sti_mode_config_init(struct drm_device *dev) @@ -352,9 +274,6 @@ static int sti_init(struct drm_device *ddev) dev_set_drvdata(ddev->dev, ddev); private->drm_dev = ddev;
- mutex_init(&private->commit.lock); - INIT_WORK(&private->commit.work, sti_atomic_work); - drm_mode_config_init(ddev);
sti_mode_config_init(ddev); diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h index 78ebe5e..f9276cd 100644 --- a/drivers/gpu/drm/sti/sti_drv.h +++ b/drivers/gpu/drm/sti/sti_drv.h @@ -25,12 +25,6 @@ struct sti_private { struct drm_property *plane_zorder_property; struct drm_device *drm_dev; struct drm_fbdev_cma *fbdev; - - struct { - struct drm_atomic_state *state; - struct work_struct work; - struct mutex lock; - } commit; };
extern struct platform_driver sti_tvout_driver;
Acked-by: Vincent Abriou vincent.abriou@st.com
On 01/12/2017 05:27 PM, Fabien Dessenne wrote:
Since nonblocking atomic commits are now supported, the driver can now use drm_atomic_helper_commit().
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com
drivers/gpu/drm/sti/sti_drv.c | 83 +------------------------------------------ drivers/gpu/drm/sti/sti_drv.h | 6 ---- 2 files changed, 1 insertion(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index ff71e25..9a9b9a2 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -150,52 +150,6 @@ static void sti_drm_dbg_cleanup(struct drm_minor *minor) 1, minor); }
-static void sti_atomic_schedule(struct sti_private *private,
struct drm_atomic_state *state)
-{
- private->commit.state = state;
- schedule_work(&private->commit.work);
-}
-static void sti_atomic_complete(struct sti_private *private,
struct drm_atomic_state *state)
-{
- struct drm_device *drm = private->drm_dev;
- /*
* Everything below can be run asynchronously without the need to grab
* any modeset locks at all under one condition: It must be guaranteed
* that the asynchronous work has either been cancelled (if the driver
* supports it, which at least requires that the framebuffers get
* cleaned up with drm_atomic_helper_cleanup_planes()) or completed
* before the new state gets committed on the software side with
* drm_atomic_helper_swap_state().
*
* This scheme allows new atomic state updates to be prepared and
* checked in parallel to the asynchronous completion of the previous
* update. Which is important since compositors need to figure out the
* composition of the next frame right after having submitted the
* current layout.
*/
- drm_atomic_helper_commit_modeset_disables(drm, state);
- drm_atomic_helper_commit_planes(drm, state, 0);
- drm_atomic_helper_commit_modeset_enables(drm, state);
- drm_atomic_helper_wait_for_vblanks(drm, state);
- drm_atomic_helper_cleanup_planes(drm, state);
- drm_atomic_state_put(state);
-}
-static void sti_atomic_work(struct work_struct *work) -{
- struct sti_private *private = container_of(work,
struct sti_private, commit.work);
- sti_atomic_complete(private, private->commit.state);
-}
static int sti_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) { @@ -216,38 +170,6 @@ static int sti_atomic_check(struct drm_device *dev, return ret; }
-static int sti_atomic_commit(struct drm_device *drm,
struct drm_atomic_state *state, bool nonblock)
-{
- struct sti_private *private = drm->dev_private;
- int err;
- err = drm_atomic_helper_prepare_planes(drm, state);
- if (err)
return err;
- /* serialize outstanding nonblocking commits */
- mutex_lock(&private->commit.lock);
- flush_work(&private->commit.work);
- /*
* This is the point of no return - everything below never fails except
* when the hw goes bonghits. Which means we can commit the new state on
* the software side now.
*/
- drm_atomic_helper_swap_state(state, true);
- drm_atomic_state_get(state);
- if (nonblock)
sti_atomic_schedule(private, state);
- else
sti_atomic_complete(private, state);
- mutex_unlock(&private->commit.lock);
- return 0;
-}
static void sti_output_poll_changed(struct drm_device *ddev) { struct sti_private *private = ddev->dev_private; @@ -271,7 +193,7 @@ static const struct drm_mode_config_funcs sti_mode_config_funcs = { .fb_create = drm_fb_cma_create, .output_poll_changed = sti_output_poll_changed, .atomic_check = sti_atomic_check,
- .atomic_commit = sti_atomic_commit,
- .atomic_commit = drm_atomic_helper_commit,
};
static void sti_mode_config_init(struct drm_device *dev) @@ -352,9 +274,6 @@ static int sti_init(struct drm_device *ddev) dev_set_drvdata(ddev->dev, ddev); private->drm_dev = ddev;
mutex_init(&private->commit.lock);
INIT_WORK(&private->commit.work, sti_atomic_work);
drm_mode_config_init(ddev);
sti_mode_config_init(ddev);
diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h index 78ebe5e..f9276cd 100644 --- a/drivers/gpu/drm/sti/sti_drv.h +++ b/drivers/gpu/drm/sti/sti_drv.h @@ -25,12 +25,6 @@ struct sti_private { struct drm_property *plane_zorder_property; struct drm_device *drm_dev; struct drm_fbdev_cma *fbdev;
- struct {
struct drm_atomic_state *state;
struct work_struct work;
struct mutex lock;
- } commit;
};
extern struct platform_driver sti_tvout_driver;
Use drm-core to handle event. This is required to be able to use the nonblocking helpers.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com --- drivers/gpu/drm/sti/sti_crtc.c | 46 +++++++++++++---------------------------- drivers/gpu/drm/sti/sti_mixer.h | 2 -- 2 files changed, 14 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c index e992bed..d45a433 100644 --- a/drivers/gpu/drm/sti/sti_crtc.c +++ b/drivers/gpu/drm/sti/sti_crtc.c @@ -134,21 +134,6 @@ sti_crtc_mode_set_nofb(struct drm_crtc *crtc) sti_crtc_mode_set(crtc, &crtc->state->adjusted_mode); }
-static void sti_crtc_atomic_begin(struct drm_crtc *crtc, - struct drm_crtc_state *old_crtc_state) -{ - struct sti_mixer *mixer = to_sti_mixer(crtc); - - if (crtc->state->event) { - crtc->state->event->pipe = drm_crtc_index(crtc); - - WARN_ON(drm_crtc_vblank_get(crtc) != 0); - - mixer->pending_event = crtc->state->event; - crtc->state->event = NULL; - } -} - static void sti_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { @@ -156,6 +141,8 @@ static void sti_crtc_atomic_flush(struct drm_crtc *crtc, struct sti_mixer *mixer = to_sti_mixer(crtc); struct sti_compositor *compo = dev_get_drvdata(mixer->dev); struct drm_plane *p; + struct drm_pending_vblank_event *event; + unsigned long flags;
DRM_DEBUG_DRIVER("\n");
@@ -220,13 +207,24 @@ static void sti_crtc_atomic_flush(struct drm_crtc *crtc, break; } } + + event = crtc->state->event; + if (event) { + crtc->state->event = NULL; + + spin_lock_irqsave(&crtc->dev->event_lock, flags); + if (drm_crtc_vblank_get(crtc) == 0) + drm_crtc_arm_vblank_event(crtc, event); + else + drm_crtc_send_vblank_event(crtc, event); + spin_unlock_irqrestore(&crtc->dev->event_lock, flags); + } }
static const struct drm_crtc_helper_funcs sti_crtc_helper_funcs = { .enable = sti_crtc_enable, .disable = sti_crtc_disabling, .mode_set_nofb = sti_crtc_mode_set_nofb, - .atomic_begin = sti_crtc_atomic_begin, .atomic_flush = sti_crtc_atomic_flush, };
@@ -250,7 +248,6 @@ int sti_crtc_vblank_cb(struct notifier_block *nb, struct sti_compositor *compo; struct drm_crtc *crtc = data; struct sti_mixer *mixer; - unsigned long flags; struct sti_private *priv; unsigned int pipe;
@@ -267,14 +264,6 @@ int sti_crtc_vblank_cb(struct notifier_block *nb,
drm_crtc_handle_vblank(crtc);
- spin_lock_irqsave(&crtc->dev->event_lock, flags); - if (mixer->pending_event) { - drm_crtc_send_vblank_event(crtc, mixer->pending_event); - drm_crtc_vblank_put(crtc); - mixer->pending_event = NULL; - } - spin_unlock_irqrestore(&crtc->dev->event_lock, flags); - if (mixer->status == STI_MIXER_DISABLING) { struct drm_plane *p;
@@ -317,19 +306,12 @@ void sti_crtc_disable_vblank(struct drm_device *drm_dev, unsigned int pipe) struct sti_private *priv = drm_dev->dev_private; struct sti_compositor *compo = priv->compo; struct notifier_block *vtg_vblank_nb = &compo->vtg_vblank_nb[pipe]; - struct drm_crtc *crtc = &compo->mixer[pipe]->drm_crtc; struct sti_vtg *vtg = compo->vtg[pipe];
DRM_DEBUG_DRIVER("\n");
if (sti_vtg_unregister_client(vtg, vtg_vblank_nb)) DRM_DEBUG_DRIVER("Warning: cannot unregister VTG notifier\n"); - - /* free the resources of the pending requests */ - if (compo->mixer[pipe]->pending_event) { - drm_crtc_vblank_put(crtc); - compo->mixer[pipe]->pending_event = NULL; - } }
static int sti_crtc_late_register(struct drm_crtc *crtc) diff --git a/drivers/gpu/drm/sti/sti_mixer.h b/drivers/gpu/drm/sti/sti_mixer.h index 830a3c4..e64a00e 100644 --- a/drivers/gpu/drm/sti/sti_mixer.h +++ b/drivers/gpu/drm/sti/sti_mixer.h @@ -28,7 +28,6 @@ enum sti_mixer_status { * @regs: mixer registers * @id: id of the mixer * @drm_crtc: crtc object link to the mixer - * @pending_event: set if a flip event is pending on crtc * @status: to know the status of the mixer */ struct sti_mixer { @@ -36,7 +35,6 @@ struct sti_mixer { void __iomem *regs; int id; struct drm_crtc drm_crtc; - struct drm_pending_vblank_event *pending_event; enum sti_mixer_status status; };
Acked-by: Vincent Abriou vincent.abriou@st.com
On 01/12/2017 05:27 PM, Fabien Dessenne wrote:
Use drm-core to handle event. This is required to be able to use the nonblocking helpers.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com
drivers/gpu/drm/sti/sti_crtc.c | 46 +++++++++++++---------------------------- drivers/gpu/drm/sti/sti_mixer.h | 2 -- 2 files changed, 14 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c index e992bed..d45a433 100644 --- a/drivers/gpu/drm/sti/sti_crtc.c +++ b/drivers/gpu/drm/sti/sti_crtc.c @@ -134,21 +134,6 @@ sti_crtc_mode_set_nofb(struct drm_crtc *crtc) sti_crtc_mode_set(crtc, &crtc->state->adjusted_mode); }
-static void sti_crtc_atomic_begin(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
-{
- struct sti_mixer *mixer = to_sti_mixer(crtc);
- if (crtc->state->event) {
crtc->state->event->pipe = drm_crtc_index(crtc);
WARN_ON(drm_crtc_vblank_get(crtc) != 0);
mixer->pending_event = crtc->state->event;
crtc->state->event = NULL;
- }
-}
static void sti_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { @@ -156,6 +141,8 @@ static void sti_crtc_atomic_flush(struct drm_crtc *crtc, struct sti_mixer *mixer = to_sti_mixer(crtc); struct sti_compositor *compo = dev_get_drvdata(mixer->dev); struct drm_plane *p;
struct drm_pending_vblank_event *event;
unsigned long flags;
DRM_DEBUG_DRIVER("\n");
@@ -220,13 +207,24 @@ static void sti_crtc_atomic_flush(struct drm_crtc *crtc, break; } }
- event = crtc->state->event;
- if (event) {
crtc->state->event = NULL;
spin_lock_irqsave(&crtc->dev->event_lock, flags);
if (drm_crtc_vblank_get(crtc) == 0)
drm_crtc_arm_vblank_event(crtc, event);
else
drm_crtc_send_vblank_event(crtc, event);
spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
- }
}
static const struct drm_crtc_helper_funcs sti_crtc_helper_funcs = { .enable = sti_crtc_enable, .disable = sti_crtc_disabling, .mode_set_nofb = sti_crtc_mode_set_nofb,
- .atomic_begin = sti_crtc_atomic_begin, .atomic_flush = sti_crtc_atomic_flush,
};
@@ -250,7 +248,6 @@ int sti_crtc_vblank_cb(struct notifier_block *nb, struct sti_compositor *compo; struct drm_crtc *crtc = data; struct sti_mixer *mixer;
- unsigned long flags; struct sti_private *priv; unsigned int pipe;
@@ -267,14 +264,6 @@ int sti_crtc_vblank_cb(struct notifier_block *nb,
drm_crtc_handle_vblank(crtc);
- spin_lock_irqsave(&crtc->dev->event_lock, flags);
- if (mixer->pending_event) {
drm_crtc_send_vblank_event(crtc, mixer->pending_event);
drm_crtc_vblank_put(crtc);
mixer->pending_event = NULL;
- }
- spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
- if (mixer->status == STI_MIXER_DISABLING) { struct drm_plane *p;
@@ -317,19 +306,12 @@ void sti_crtc_disable_vblank(struct drm_device *drm_dev, unsigned int pipe) struct sti_private *priv = drm_dev->dev_private; struct sti_compositor *compo = priv->compo; struct notifier_block *vtg_vblank_nb = &compo->vtg_vblank_nb[pipe];
struct drm_crtc *crtc = &compo->mixer[pipe]->drm_crtc; struct sti_vtg *vtg = compo->vtg[pipe];
DRM_DEBUG_DRIVER("\n");
if (sti_vtg_unregister_client(vtg, vtg_vblank_nb)) DRM_DEBUG_DRIVER("Warning: cannot unregister VTG notifier\n");
/* free the resources of the pending requests */
if (compo->mixer[pipe]->pending_event) {
drm_crtc_vblank_put(crtc);
compo->mixer[pipe]->pending_event = NULL;
}
}
static int sti_crtc_late_register(struct drm_crtc *crtc) diff --git a/drivers/gpu/drm/sti/sti_mixer.h b/drivers/gpu/drm/sti/sti_mixer.h index 830a3c4..e64a00e 100644 --- a/drivers/gpu/drm/sti/sti_mixer.h +++ b/drivers/gpu/drm/sti/sti_mixer.h @@ -28,7 +28,6 @@ enum sti_mixer_status {
- @regs: mixer registers
- @id: id of the mixer
- @drm_crtc: crtc object link to the mixer
*/
- @pending_event: set if a flip event is pending on crtc
- @status: to know the status of the mixer
struct sti_mixer { @@ -36,7 +35,6 @@ struct sti_mixer { void __iomem *regs; int id; struct drm_crtc drm_crtc;
- struct drm_pending_vblank_event *pending_event; enum sti_mixer_status status;
};
Fix a division by 0 case : in some cases, when the HQVDP plane is being disabled atomic_check() is called with "mode->clock = 0". In that case, do not check for scaling capabilities.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com --- drivers/gpu/drm/sti/sti_hqvdp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index f88130f..723ac30 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -1035,9 +1035,9 @@ static int sti_hqvdp_atomic_check(struct drm_plane *drm_plane, src_w = state->src_w >> 16; src_h = state->src_h >> 16;
- if (!sti_hqvdp_check_hw_scaling(hqvdp, mode, - src_w, src_h, - dst_w, dst_h)) { + if (mode->clock && !sti_hqvdp_check_hw_scaling(hqvdp, mode, + src_w, src_h, + dst_w, dst_h)) { DRM_ERROR("Scaling beyond HW capabilities\n"); return -EINVAL; }
Acked-by: Vincent Abriou vincent.abriou@st.com
On 01/12/2017 05:27 PM, Fabien Dessenne wrote:
Fix a division by 0 case : in some cases, when the HQVDP plane is being disabled atomic_check() is called with "mode->clock = 0". In that case, do not check for scaling capabilities.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com
drivers/gpu/drm/sti/sti_hqvdp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index f88130f..723ac30 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -1035,9 +1035,9 @@ static int sti_hqvdp_atomic_check(struct drm_plane *drm_plane, src_w = state->src_w >> 16; src_h = state->src_h >> 16;
- if (!sti_hqvdp_check_hw_scaling(hqvdp, mode,
src_w, src_h,
dst_w, dst_h)) {
- if (mode->clock && !sti_hqvdp_check_hw_scaling(hqvdp, mode,
src_w, src_h,
DRM_ERROR("Scaling beyond HW capabilities\n"); return -EINVAL; }dst_w, dst_h)) {
dri-devel@lists.freedesktop.org