drm/amd/display: Fix handling of scaling and underscan.
authorAndrey Grodzovsky <Andrey.Grodzovsky@amd.com>
Mon, 22 May 2017 21:55:38 +0000 (17:55 -0400)
committerAlex Deucher <alexander.deucher@amd.com>
Tue, 26 Sep 2017 22:07:32 +0000 (18:07 -0400)
Summury of changes:

1: Both in check and commit Connector properties were handled as
   part of for_each(crtc) loop while they shoud have been handled
   in a dedicated for_each(connector)
   loop since they are connector properties. Moved.

2: Removed hacky plane add in amdgpu_dm_connector_atomic_set_property
   to force iteration on plane forconnector property. This was
   causing double call to commit_surface_for_stream both in crtc loop
   and plane loop.
3: Remove middleman DC interface and  call dc_commit_surfaces_to_stream
   directly to increase code clarity.

Remove it from atomic_commit.

Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c

index 3c1baa2..4da5a71 100644 (file)
@@ -682,9 +682,8 @@ struct amdgpu_connector *aconnector_from_drm_crtc_id(
 static void update_stream_scaling_settings(
                const struct drm_display_mode *mode,
                const struct dm_connector_state *dm_state,
-               const struct dc_stream *stream)
+               struct dc_stream *stream)
 {
-       struct amdgpu_device *adev = dm_state->base.crtc->dev->dev_private;
        enum amdgpu_rmx_type rmx_type;
 
        struct rect src = { 0 }; /* viewport in composition space*/
@@ -726,7 +725,8 @@ static void update_stream_scaling_settings(
                dst.height -= dm_state->underscan_vborder;
        }
 
-       adev->dm.dc->stream_funcs.stream_update_scaling(adev->dm.dc, stream, &src, &dst);
+       stream->src = src;
+       stream->dst = dst;
 
        DRM_DEBUG_KMS("Destination Rectangle x:%d  y:%d  width:%d  height:%d\n",
                        dst.x, dst.y, dst.width, dst.height);
@@ -1135,9 +1135,6 @@ int amdgpu_dm_connector_atomic_set_property(
        struct dm_connector_state *dm_new_state =
                to_dm_connector_state(connector_state);
 
-       struct drm_crtc_state *new_crtc_state;
-       struct drm_crtc *crtc;
-       int i;
        int ret = -EINVAL;
 
        if (property == dev->mode_config.scaling_mode_property) {
@@ -1175,32 +1172,6 @@ int amdgpu_dm_connector_atomic_set_property(
                ret = 0;
        }
 
-       for_each_crtc_in_state(
-               connector_state->state,
-               crtc,
-               new_crtc_state,
-               i) {
-
-               if (crtc == connector_state->crtc) {
-                       struct drm_plane_state *plane_state;
-
-                       /*
-                        * Bit of magic done here. We need to ensure
-                        * that planes get update after mode is set.
-                        * So, we need to add primary plane to state,
-                        * and this way atomic_update would be called
-                        * for it
-                        */
-                       plane_state =
-                               drm_atomic_get_plane_state(
-                                       connector_state->state,
-                                       crtc->primary);
-
-                       if (!plane_state)
-                               return -EINVAL;
-               }
-       }
-
        return ret;
 }
 
@@ -2401,28 +2372,19 @@ static void amdgpu_dm_commit_surfaces(struct drm_atomic_state *state,
                struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(crtc);
                struct drm_framebuffer *fb = plane_state->fb;
                struct drm_connector *connector;
-               struct dm_connector_state *dm_state = NULL;
-
-               enum dm_commit_action action;
+               struct dm_connector_state *con_state = NULL;
                bool pflip_needed;
 
                if (!fb || !crtc || !crtc->state->active)
                        continue;
 
-               action = get_dm_commit_action(crtc->state);
-
-               /*
-                * TODO - TO decide if it's a flip or surface update
-                * stop relying on allow_modeset flag and query DC
-                * using dc_check_update_surfaces_for_stream.
-                */
                pflip_needed = !state->allow_modeset;
                if (!pflip_needed) {
                        list_for_each_entry(connector,
                                            &dev->mode_config.connector_list,
                                            head) {
                                if (connector->state->crtc == crtc) {
-                                       dm_state = to_dm_connector_state(
+                                       con_state = to_dm_connector_state(
                                                        connector->state);
                                        break;
                                }
@@ -2442,8 +2404,10 @@ static void amdgpu_dm_commit_surfaces(struct drm_atomic_state *state,
                         * Also it should be needed when used with actual
                         * drm_atomic_commit ioctl in future
                         */
-                       if (!dm_state)
+                       if (!con_state)
                                continue;
+
+
                        if (crtc == pcrtc) {
                                add_surface(dm->dc, crtc, plane,
                                            &dc_surfaces_constructed[planes_count]);
@@ -2495,7 +2459,8 @@ void amdgpu_dm_atomic_commit_tail(
        const struct dc_stream *new_stream;
        unsigned long flags;
        bool wait_for_vblank = true;
-
+       struct drm_connector *connector;
+       struct drm_connector_state *old_conn_state;
 
        drm_atomic_helper_update_legacy_modeset_state(dev, state);
 
@@ -2575,21 +2540,6 @@ void amdgpu_dm_atomic_commit_tail(
 
                        break;
                }
-
-               case DM_COMMIT_ACTION_NOTHING: {
-                       struct dm_connector_state *dm_state = NULL;
-
-                       if (!aconnector)
-                               break;
-
-                       dm_state = to_dm_connector_state(aconnector->base.state);
-
-                       /* Scaling update */
-                       update_stream_scaling_settings(&crtc->state->mode,
-                                       dm_state, acrtc->stream);
-
-                       break;
-               }
                case DM_COMMIT_ACTION_DPMS_OFF:
                case DM_COMMIT_ACTION_RESET:
                        DRM_INFO("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
@@ -2597,9 +2547,48 @@ void amdgpu_dm_atomic_commit_tail(
                        if (acrtc->stream)
                                remove_stream(adev, acrtc);
                        break;
+
+               /*TODO retire */
+               case DM_COMMIT_ACTION_NOTHING:
+                       continue;
                } /* switch() */
        } /* for_each_crtc_in_state() */
 
+       /* Handle scaling and undersacn changes*/
+       for_each_connector_in_state(state, connector, old_conn_state, i) {
+               struct amdgpu_connector *aconnector = to_amdgpu_connector(connector);
+               struct dm_connector_state *con_new_state =
+                               to_dm_connector_state(aconnector->base.state);
+               struct dm_connector_state *con_old_state =
+                               to_dm_connector_state(old_conn_state);
+               struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc);
+               const struct dc_stream_status *status = NULL;
+
+               /* Skip any modesets/resets */
+               if (!acrtc ||
+                       get_dm_commit_action(acrtc->base.state) != DM_COMMIT_ACTION_NOTHING)
+                       continue;
+
+               /* Skip any thing not scale or underscan chnages */
+               if (!is_scaling_state_different(con_new_state, con_old_state))
+                       continue;
+
+               update_stream_scaling_settings(&con_new_state->base.crtc->mode,
+                               con_new_state, (struct dc_stream *)acrtc->stream);
+
+               status = dc_stream_get_status(acrtc->stream);
+               WARN_ON(!status);
+               WARN_ON(!status->surface_count);
+
+               /*TODO How it works with MPO ?*/
+               if (!dc_commit_surfaces_to_stream(
+                               dm->dc,
+                               (const struct dc_surface **)status->surfaces,
+                               status->surface_count,
+                               acrtc->stream))
+                       dm_error("%s: Failed to update stream scaling!\n", __func__);
+       }
+
        list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 
                struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
@@ -2937,6 +2926,8 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
        struct dc *dc = adev->dm.dc;
        bool need_to_validate = false;
        struct validate_context *context;
+       struct drm_connector *connector;
+       struct drm_connector_state *conn_state;
        /*
         * This bool will be set for true for any modeset/reset
         * or surface update which implies non fast surfae update.
@@ -3022,52 +3013,6 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
                        break;
                }
 
-               case DM_COMMIT_ACTION_NOTHING: {
-                       const struct drm_connector *drm_connector = NULL;
-                       struct drm_connector_state *conn_state = NULL;
-                       struct dm_connector_state *dm_state = NULL;
-                       struct dm_connector_state *old_dm_state = NULL;
-                       struct dc_stream *new_stream;
-
-                       if (!aconnector)
-                               break;
-
-                       for_each_connector_in_state(
-                               state, drm_connector, conn_state, j) {
-                               if (&aconnector->base == drm_connector)
-                                       break;
-                       }
-
-                       old_dm_state = to_dm_connector_state(drm_connector->state);
-                       dm_state = to_dm_connector_state(conn_state);
-
-                       /* Support underscan adjustment*/
-                       if (!is_scaling_state_different(dm_state, old_dm_state))
-                               break;
-
-                       new_stream = create_stream_for_sink(aconnector, &crtc_state->mode, dm_state);
-
-                       if (!new_stream) {
-                               DRM_ERROR("%s: Failed to create new stream for crtc %d\n",
-                                               __func__, acrtc->base.base.id);
-                               break;
-                       }
-
-                       new_streams[new_stream_count] = new_stream;
-                       set_count = update_in_val_sets_stream(
-                                       set,
-                                       crtc_set,
-                                       set_count,
-                                       acrtc->stream,
-                                       new_stream,
-                                       crtc);
-
-                       new_stream_count++;
-                       need_to_validate = true;
-                       wait_for_prev_commits = true;
-
-                       break;
-               }
                case DM_COMMIT_ACTION_DPMS_OFF:
                case DM_COMMIT_ACTION_RESET:
                        /* i.e. reset mode */
@@ -3079,6 +3024,10 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
                                wait_for_prev_commits = true;
                        }
                        break;
+
+               /*TODO retire */
+               case DM_COMMIT_ACTION_NOTHING:
+                       continue;
                }
 
                /*
@@ -3095,6 +3044,50 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
                ret = -EINVAL;
        }
 
+       /* Check scaling and undersacn changes*/
+       for_each_connector_in_state(state, connector, conn_state, i) {
+               struct amdgpu_connector *aconnector = to_amdgpu_connector(connector);
+               struct dm_connector_state *con_old_state =
+                               to_dm_connector_state(aconnector->base.state);
+               struct dm_connector_state *con_new_state =
+                                               to_dm_connector_state(conn_state);
+               struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc);
+               struct dc_stream *new_stream;
+
+               /* Skip any modesets/resets */
+               if (!acrtc ||
+                       get_dm_commit_action(acrtc->base.state) != DM_COMMIT_ACTION_NOTHING)
+                       continue;
+
+               /* Skip any thing not scale or underscan chnages */
+               if (!is_scaling_state_different(con_new_state, con_old_state))
+                       continue;
+
+               new_stream = create_stream_for_sink(
+                               aconnector,
+                               &acrtc->base.state->mode,
+                               con_new_state);
+
+               if (!new_stream) {
+                       DRM_ERROR("%s: Failed to create new stream for crtc %d\n",
+                                       __func__, acrtc->base.base.id);
+                       continue;
+               }
+
+               new_streams[new_stream_count] = new_stream;
+               set_count = update_in_val_sets_stream(
+                               set,
+                               crtc_set,
+                               set_count,
+                               acrtc->stream,
+                               new_stream,
+                               &acrtc->base);
+
+               new_stream_count++;
+               need_to_validate = true;
+               wait_for_prev_commits = true;
+       }
+
        for (i = 0; i < set_count; i++) {
                for_each_plane_in_state(state, plane, plane_state, j) {
                        struct drm_crtc *crtc = plane_state->crtc;