[PATCH 10/10] drm/exynos: atomic dpms support
Joonyoung Shim
jy0922.shim at samsung.com
Thu Apr 2 02:02:02 PDT 2015
Hi,
On 03/31/2015 04:11 AM, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>
> Run dpms operations through the atomic intefaces. This basically removes
> the .dpms() callback from econders and crtcs and use .disable() and
> .enable() to turn the crtc on and off.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> ---
> drivers/gpu/drm/exynos/exynos_dp_core.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 99 ++++++++++++++++-------------
> drivers/gpu/drm/exynos/exynos_drm_dpi.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_drv.h | 4 +-
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_encoder.c | 27 ++------
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 3 -
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 1 -
> drivers/gpu/drm/exynos/exynos_drm_vidi.c | 2 +-
> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> 10 files changed, 67 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index 6704d5c..e030d16 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -949,7 +949,7 @@ static void exynos_dp_connector_destroy(struct drm_connector *connector)
> }
>
> static struct drm_connector_funcs exynos_dp_connector_funcs = {
> - .dpms = drm_helper_connector_dpms,
> + .dpms = drm_atomic_helper_connector_dpms,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .detect = exynos_dp_detect,
> .destroy = exynos_dp_connector_destroy,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index 0db7b91..b493993 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -22,51 +22,57 @@
> #include "exynos_drm_encoder.h"
> #include "exynos_drm_plane.h"
>
> -static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
> +static void exynos_drm_crtc_enable(struct drm_crtc *crtc)
> {
> struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> + struct exynos_drm_plane *exynos_plane = to_exynos_plane(crtc->primary);
>
> - DRM_DEBUG_KMS("crtc[%d] mode[%d]\n", crtc->base.id, mode);
> -
> - if (exynos_crtc->dpms == mode) {
> - DRM_DEBUG_KMS("desired dpms mode is same as previous one.\n");
> + if (WARN_ON(exynos_crtc->enabled))
I'm not sure this is really important condition to report warning.
> return;
> - }
> -
> - if (mode > DRM_MODE_DPMS_ON) {
> - /* wait for the completion of page flip. */
> - if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
> - (exynos_crtc->event == NULL), HZ/20))
> - exynos_crtc->event = NULL;
> - drm_crtc_vblank_off(crtc);
> - }
>
> if (exynos_crtc->ops->dpms)
> - exynos_crtc->ops->dpms(exynos_crtc, mode);
> + exynos_crtc->ops->dpms(exynos_crtc, DRM_MODE_DPMS_ON);
>
> - exynos_crtc->dpms = mode;
> + exynos_crtc->enabled = true;
>
> - if (mode == DRM_MODE_DPMS_ON)
> - drm_crtc_vblank_on(crtc);
> -}
> + drm_crtc_vblank_on(crtc);
>
> -static void exynos_drm_crtc_prepare(struct drm_crtc *crtc)
> -{
> - /* drm framework doesn't check NULL. */
> + if (exynos_crtc->ops->win_commit)
> + exynos_crtc->ops->win_commit(exynos_crtc, exynos_plane->zpos);
> +
> + if (exynos_crtc->ops->commit)
> + exynos_crtc->ops->commit(exynos_crtc);
> }
>
> -static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
> +static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
> {
> struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> - struct exynos_drm_plane *exynos_plane = to_exynos_plane(crtc->primary);
> + struct drm_plane *plane;
> + int ret;
> +
> + if (WARN_ON(!exynos_crtc->enabled))
> + return;
>
> - exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
> + /* wait for the completion of page flip. */
> + if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
> + (exynos_crtc->event == NULL), HZ/20))
> + exynos_crtc->event = NULL;
>
> - if (exynos_crtc->ops->win_commit)
> - exynos_crtc->ops->win_commit(exynos_crtc, exynos_plane->zpos);
> + drm_crtc_vblank_off(crtc);
>
> - if (exynos_crtc->ops->commit)
> - exynos_crtc->ops->commit(exynos_crtc);
> + if (exynos_crtc->ops->dpms)
> + exynos_crtc->ops->dpms(exynos_crtc, DRM_MODE_DPMS_OFF);
> +
> + exynos_crtc->enabled = false;
> +
> + drm_for_each_legacy_plane(plane, &crtc->dev->mode_config.plane_list) {
> + if (plane->crtc != crtc)
> + continue;
> +
> + ret = plane->funcs->disable_plane(plane);
> + if (ret)
> + DRM_ERROR("Failed to disable plane %d\n", ret);
> + }
> }
>
> static bool
> @@ -95,32 +101,36 @@ exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
> exynos_crtc->ops->commit(exynos_crtc);
> }
>
> -static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
> +static int exynos_crtc_atomic_check(struct drm_crtc *crtc,
> + struct drm_crtc_state *state)
> {
> - struct drm_plane *plane;
> + struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
> int ret;
>
> - exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
> + if (exynos_crtc->event)
> + return -EBUSY;
>
> - drm_for_each_legacy_plane(plane, &crtc->dev->mode_config.plane_list) {
> - if (plane->crtc != crtc)
> - continue;
> + if (state->event) {
> + ret = drm_vblank_get(crtc->dev, exynos_crtc->pipe);
> + if (ret) {
> + DRM_ERROR("failed to acquire vblank counter\n");
> + return ret;
> + }
>
> - ret = plane->funcs->disable_plane(plane);
> - if (ret)
> - DRM_ERROR("Failed to disable plane %d\n", ret);
> + exynos_crtc->event = state->event;
> }
> +
> + return 0;
> }
>
> static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
> - .dpms = exynos_drm_crtc_dpms,
> - .prepare = exynos_drm_crtc_prepare,
> - .commit = exynos_drm_crtc_commit,
> + .enable = exynos_drm_crtc_enable,
> + .disable = exynos_drm_crtc_disable,
By this change, funcs->dpms should be changed to funcs->disable on
hdmi_dpms function of drivers/gpu/drm/exynos/exynos_hdmi.c or any idea?
case DRM_MODE_DPMS_OFF:
/*
* The SFRs of VP and Mixer are updated by Vertical Sync of
* Timing generator which is a part of HDMI so the sequence
* to disable TV Subsystem should be as following,
* VP -> Mixer -> HDMI
*
* Below codes will try to disable Mixer and VP(if used)
* prior to disabling HDMI.
*/
if (crtc)
funcs = crtc->helper_private;
if (funcs && funcs->dpms)
(*funcs->dpms)(crtc, mode);
hdmi_poweroff(hdata);
break;
> .mode_fixup = exynos_drm_crtc_mode_fixup,
> .mode_set = drm_helper_crtc_mode_set,
> .mode_set_nofb = exynos_drm_crtc_mode_set_nofb,
> .mode_set_base = drm_helper_crtc_mode_set_base,
> - .disable = exynos_drm_crtc_disable,
> + .atomic_check = exynos_crtc_atomic_check,
> };
>
> static void exynos_drm_crtc_destroy(struct drm_crtc *crtc)
> @@ -162,7 +172,6 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
>
> init_waitqueue_head(&exynos_crtc->pending_flip_queue);
>
> - exynos_crtc->dpms = DRM_MODE_DPMS_OFF;
> exynos_crtc->pipe = pipe;
> exynos_crtc->type = type;
> exynos_crtc->ops = ops;
> @@ -193,7 +202,7 @@ int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int pipe)
> struct exynos_drm_crtc *exynos_crtc =
> to_exynos_crtc(private->crtc[pipe]);
>
> - if (exynos_crtc->dpms != DRM_MODE_DPMS_ON)
> + if (!exynos_crtc->enabled)
> return -EPERM;
>
> if (exynos_crtc->ops->enable_vblank)
> @@ -208,7 +217,7 @@ void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe)
> struct exynos_drm_crtc *exynos_crtc =
> to_exynos_crtc(private->crtc[pipe]);
>
> - if (exynos_crtc->dpms != DRM_MODE_DPMS_ON)
> + if (!exynos_crtc->enabled)
> return;
>
> if (exynos_crtc->ops->disable_vblank)
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> index ced5c23..6dc328e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> @@ -60,7 +60,7 @@ static void exynos_dpi_connector_destroy(struct drm_connector *connector)
> }
>
> static struct drm_connector_funcs exynos_dpi_connector_funcs = {
> - .dpms = drm_helper_connector_dpms,
> + .dpms = drm_atomic_helper_connector_dpms,
> .detect = exynos_dpi_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .destroy = exynos_dpi_connector_destroy,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index a1013aa..a7b708e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -205,7 +205,7 @@ struct exynos_drm_crtc_ops {
> * drm framework doesn't support multiple irq yet.
> * we can refer to the crtc to current hardware interrupt occurred through
> * this pipe value.
> - * @dpms: store the crtc dpms value
> + * @enabled: if the crtc is enabled or not
> * @event: vblank event that is currently queued for flip
> * @ops: pointer to callbacks for exynos drm specific functionality
> * @ctx: A pointer to the crtc's implementation specific context
> @@ -214,7 +214,7 @@ struct exynos_drm_crtc {
> struct drm_crtc base;
> enum exynos_drm_output_type type;
> unsigned int pipe;
> - unsigned int dpms;
> + bool enabled;
> wait_queue_head_t pending_flip_queue;
> struct drm_pending_vblank_event *event;
> struct exynos_drm_crtc_ops *ops;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 6e9c2c3..28233a5 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1458,7 +1458,7 @@ static void exynos_dsi_connector_destroy(struct drm_connector *connector)
> }
>
> static struct drm_connector_funcs exynos_dsi_connector_funcs = {
> - .dpms = drm_helper_connector_dpms,
> + .dpms = drm_atomic_helper_connector_dpms,
> .detect = exynos_dsi_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .destroy = exynos_dsi_connector_destroy,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> index 57de0bd..0648ba4 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> @@ -32,17 +32,6 @@ struct exynos_drm_encoder {
> struct exynos_drm_display *display;
> };
>
> -static void exynos_drm_encoder_dpms(struct drm_encoder *encoder, int mode)
> -{
> - struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
> - struct exynos_drm_display *display = exynos_encoder->display;
> -
> - DRM_DEBUG_KMS("encoder dpms: %d\n", mode);
> -
> - if (display->ops->dpms)
> - display->ops->dpms(display, mode);
> -}
> -
> static bool
> exynos_drm_encoder_mode_fixup(struct drm_encoder *encoder,
> const struct drm_display_mode *mode,
> @@ -76,12 +65,7 @@ static void exynos_drm_encoder_mode_set(struct drm_encoder *encoder,
> display->ops->mode_set(display, adjusted_mode);
> }
>
> -static void exynos_drm_encoder_prepare(struct drm_encoder *encoder)
> -{
> - /* drm framework doesn't check NULL. */
> -}
This function is related with below flow on disable_outputs fuction of
drivers/gpu/drm/drm_atomic_helper.c
/* Right function depends upon target state. */
if (connector->state->crtc && funcs->prepare)
funcs->prepare(encoder);
else if (funcs->disable)
funcs->disable(encoder);
else
funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
I'm not sure it's right to call funcs->disable instead of
funcs->prepare.
> -
> -static void exynos_drm_encoder_commit(struct drm_encoder *encoder)
> +static void exynos_drm_encoder_enable(struct drm_encoder *encoder)
> {
> struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
> struct exynos_drm_display *display = exynos_encoder->display;
> @@ -95,10 +79,13 @@ static void exynos_drm_encoder_commit(struct drm_encoder *encoder)
>
> static void exynos_drm_encoder_disable(struct drm_encoder *encoder)
> {
> + struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
> + struct exynos_drm_display *display = exynos_encoder->display;
> struct drm_plane *plane;
> struct drm_device *dev = encoder->dev;
>
> - exynos_drm_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
> + if (display->ops->dpms)
> + display->ops->dpms(display, DRM_MODE_DPMS_OFF);
>
> /* all planes connected to this encoder should be also disabled. */
> drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
> @@ -108,11 +95,9 @@ static void exynos_drm_encoder_disable(struct drm_encoder *encoder)
> }
>
> static struct drm_encoder_helper_funcs exynos_encoder_helper_funcs = {
> - .dpms = exynos_drm_encoder_dpms,
> .mode_fixup = exynos_drm_encoder_mode_fixup,
> .mode_set = exynos_drm_encoder_mode_set,
> - .prepare = exynos_drm_encoder_prepare,
> - .commit = exynos_drm_encoder_commit,
> + .enable = exynos_drm_encoder_enable,
> .disable = exynos_drm_encoder_disable,
> };
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index e71e331..e0b085b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -275,9 +275,6 @@ int exynos_drm_fbdev_init(struct drm_device *dev)
>
> }
>
> - /* disable all the possible outputs/crtcs before entering KMS mode */
> - drm_helper_disable_unused_functions(dev);
> -
I think this is not related with atomic dpms support, if this needs,
could you make another patch?
> ret = drm_fb_helper_initial_config(helper, PREFERRED_BPP);
> if (ret < 0) {
> DRM_ERROR("failed to set up hw configuration.\n");
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> index 7f8f962..5d3066d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> @@ -131,7 +131,6 @@ static int exynos_plane_atomic_check(struct drm_plane *plane,
> struct drm_plane_state *state)
> {
> struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
> - struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(plane->crtc);
> int nr;
> int i;
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index 6eddfc0..f6cad75 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -385,7 +385,7 @@ static void vidi_connector_destroy(struct drm_connector *connector)
> }
>
> static struct drm_connector_funcs vidi_connector_funcs = {
> - .dpms = drm_helper_connector_dpms,
> + .dpms = drm_atomic_helper_connector_dpms,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .detect = vidi_detect,
> .destroy = vidi_connector_destroy,
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 5b597bc..d395da0 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -1051,7 +1051,7 @@ static void hdmi_connector_destroy(struct drm_connector *connector)
> }
>
> static struct drm_connector_funcs hdmi_connector_funcs = {
> - .dpms = drm_helper_connector_dpms,
> + .dpms = drm_atomic_helper_connector_dpms,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .detect = hdmi_detect,
> .destroy = hdmi_connector_destroy,
>
Thanks.
More information about the dri-devel
mailing list