[Intel-gfx] [PATCH 03/16] drm/atomic: Move __drm_atomic_helper_disable_plane/set_config()
Noralf Trønnes
noralf at tronnes.org
Wed Mar 27 14:27:48 UTC 2019
Den 27.03.2019 14.55, skrev Daniel Vetter:
> On Wed, Mar 27, 2019 at 2:42 PM Noralf Trønnes <noralf at tronnes.org> wrote:
>>
>>
>>
>> Den 26.03.2019 21.48, skrev Daniel Vetter:
>>> On Tue, Mar 26, 2019 at 06:55:33PM +0100, Noralf Trønnes wrote:
>>>> Prepare for moving drm_fb_helper modesetting code to drm_client.
>>>> drm_client will be linked to drm.ko, so move
>>>> __drm_atomic_helper_disable_plane() and __drm_atomic_helper_set_config()
>>>> out of drm_kms_helper.ko.
>>>>
>>>> While at it, fix two checkpatch complaints:
>>>> - WARNING: Block comments use a trailing */ on a separate line
>>>> - CHECK: Alignment should match open parenthesis
>>>>
>>>> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
>>>
>>> Triggers a bit my midlayer ocd, but I can't come up with a better idea
>>> either.
>>>
>>> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>>
>>
>> The Intel CI informed me that this patch didn't apply..
>>
>> Turns out there's a patch in drm-next by Sean:
>>
>> drm: Merge __drm_atomic_helper_disable_all() into
>> drm_atomic_helper_disable_all()
>>
>> https://cgit.freedesktop.org/drm/drm/commit?id=37406a60fac7de336dec331a8707a94462ac5a5e
>>
>> Should I move the whole function with the kerneldoc to drm_atomic.c, or
>> should I keep the doc and function in drm_atomic_helper.c and just
>> recreate __drm_atomic_helper_disable_all() and call that ?
>
> So my longterm goal with all these compat helpers is to move them into
> the core anyway, and make them at least the default, if not only the
> only accepted choice for atomic drivers. We've already done that for
> the dpms hook. We can't make it the enforced default yet for planes
> because of cursors, but I think we could just move both of these into
> the core (disable_plane and update_plane), make it the default option,
> and remove all driver users that just set it as the default hook.
>
> Would need to be renamed to something like drm_atomic_default_* ofc.
>
> Not sure why you're talking about disable_all here though, since that
> doesn't seem to be touched in this patch at all?
Sorry my bad, I was just confused. Looking at it again, I see that I can
just rebase on top of that patch.
Noralf.
> -Daniel
>
>>
>> Noralf.
>>
>>>> ---
>>>> drivers/gpu/drm/drm_atomic.c | 168 ++++++++++++++++++++++++++++
>>>> drivers/gpu/drm/drm_atomic_helper.c | 164 ---------------------------
>>>> drivers/gpu/drm/drm_crtc_internal.h | 5 +
>>>> include/drm/drm_atomic_helper.h | 4 -
>>>> 4 files changed, 173 insertions(+), 168 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>>> index 5eb40130fafb..c3a9ffbf2310 100644
>>>> --- a/drivers/gpu/drm/drm_atomic.c
>>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>>> @@ -1130,6 +1130,174 @@ int drm_atomic_nonblocking_commit(struct drm_atomic_state *state)
>>>> }
>>>> EXPORT_SYMBOL(drm_atomic_nonblocking_commit);
>>>>
>>>> +/* just used from drm-client and atomic-helper: */
>>>> +int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
>>>> + struct drm_plane_state *plane_state)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
>>>> + if (ret != 0)
>>>> + return ret;
>>>> +
>>>> + drm_atomic_set_fb_for_plane(plane_state, NULL);
>>>> + plane_state->crtc_x = 0;
>>>> + plane_state->crtc_y = 0;
>>>> + plane_state->crtc_w = 0;
>>>> + plane_state->crtc_h = 0;
>>>> + plane_state->src_x = 0;
>>>> + plane_state->src_y = 0;
>>>> + plane_state->src_w = 0;
>>>> + plane_state->src_h = 0;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(__drm_atomic_helper_disable_plane);
>>>> +
>>>> +static int update_output_state(struct drm_atomic_state *state,
>>>> + struct drm_mode_set *set)
>>>> +{
>>>> + struct drm_device *dev = set->crtc->dev;
>>>> + struct drm_crtc *crtc;
>>>> + struct drm_crtc_state *new_crtc_state;
>>>> + struct drm_connector *connector;
>>>> + struct drm_connector_state *new_conn_state;
>>>> + int ret, i;
>>>> +
>>>> + ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
>>>> + state->acquire_ctx);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + /* First disable all connectors on the target crtc. */
>>>> + ret = drm_atomic_add_affected_connectors(state, set->crtc);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + for_each_new_connector_in_state(state, connector, new_conn_state, i) {
>>>> + if (new_conn_state->crtc == set->crtc) {
>>>> + ret = drm_atomic_set_crtc_for_connector(new_conn_state,
>>>> + NULL);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + /* Make sure legacy setCrtc always re-trains */
>>>> + new_conn_state->link_status = DRM_LINK_STATUS_GOOD;
>>>> + }
>>>> + }
>>>> +
>>>> + /* Then set all connectors from set->connectors on the target crtc */
>>>> + for (i = 0; i < set->num_connectors; i++) {
>>>> + new_conn_state = drm_atomic_get_connector_state(state,
>>>> + set->connectors[i]);
>>>> + if (IS_ERR(new_conn_state))
>>>> + return PTR_ERR(new_conn_state);
>>>> +
>>>> + ret = drm_atomic_set_crtc_for_connector(new_conn_state,
>>>> + set->crtc);
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>> +
>>>> + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>>>> + /*
>>>> + * Don't update ->enable for the CRTC in the set_config request,
>>>> + * since a mismatch would indicate a bug in the upper layers.
>>>> + * The actual modeset code later on will catch any
>>>> + * inconsistencies here.
>>>> + */
>>>> + if (crtc == set->crtc)
>>>> + continue;
>>>> +
>>>> + if (!new_crtc_state->connector_mask) {
>>>> + ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
>>>> + NULL);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> + new_crtc_state->active = false;
>>>> + }
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/* just used from drm-client and atomic-helper: */
>>>> +int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>>>> + struct drm_atomic_state *state)
>>>> +{
>>>> + struct drm_crtc_state *crtc_state;
>>>> + struct drm_plane_state *primary_state;
>>>> + struct drm_crtc *crtc = set->crtc;
>>>> + int hdisplay, vdisplay;
>>>> + int ret;
>>>> +
>>>> + crtc_state = drm_atomic_get_crtc_state(state, crtc);
>>>> + if (IS_ERR(crtc_state))
>>>> + return PTR_ERR(crtc_state);
>>>> +
>>>> + primary_state = drm_atomic_get_plane_state(state, crtc->primary);
>>>> + if (IS_ERR(primary_state))
>>>> + return PTR_ERR(primary_state);
>>>> +
>>>> + if (!set->mode) {
>>>> + WARN_ON(set->fb);
>>>> + WARN_ON(set->num_connectors);
>>>> +
>>>> + ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
>>>> + if (ret != 0)
>>>> + return ret;
>>>> +
>>>> + crtc_state->active = false;
>>>> +
>>>> + ret = drm_atomic_set_crtc_for_plane(primary_state, NULL);
>>>> + if (ret != 0)
>>>> + return ret;
>>>> +
>>>> + drm_atomic_set_fb_for_plane(primary_state, NULL);
>>>> +
>>>> + goto commit;
>>>> + }
>>>> +
>>>> + WARN_ON(!set->fb);
>>>> + WARN_ON(!set->num_connectors);
>>>> +
>>>> + ret = drm_atomic_set_mode_for_crtc(crtc_state, set->mode);
>>>> + if (ret != 0)
>>>> + return ret;
>>>> +
>>>> + crtc_state->active = true;
>>>> +
>>>> + ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);
>>>> + if (ret != 0)
>>>> + return ret;
>>>> +
>>>> + drm_mode_get_hv_timing(set->mode, &hdisplay, &vdisplay);
>>>> +
>>>> + drm_atomic_set_fb_for_plane(primary_state, set->fb);
>>>> + primary_state->crtc_x = 0;
>>>> + primary_state->crtc_y = 0;
>>>> + primary_state->crtc_w = hdisplay;
>>>> + primary_state->crtc_h = vdisplay;
>>>> + primary_state->src_x = set->x << 16;
>>>> + primary_state->src_y = set->y << 16;
>>>> + if (drm_rotation_90_or_270(primary_state->rotation)) {
>>>> + primary_state->src_w = vdisplay << 16;
>>>> + primary_state->src_h = hdisplay << 16;
>>>> + } else {
>>>> + primary_state->src_w = hdisplay << 16;
>>>> + primary_state->src_h = vdisplay << 16;
>>>> + }
>>>> +
>>>> +commit:
>>>> + ret = update_output_state(state, set);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(__drm_atomic_helper_set_config);
>>>> +
>>>> void drm_atomic_print_state(const struct drm_atomic_state *state)
>>>> {
>>>> struct drm_printer p = drm_info_printer(state->dev->dev);
>>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>>> index 2453678d1186..b0d960da53cb 100644
>>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>>> @@ -2831,95 +2831,6 @@ int drm_atomic_helper_disable_plane(struct drm_plane *plane,
>>>> }
>>>> EXPORT_SYMBOL(drm_atomic_helper_disable_plane);
>>>>
>>>> -/* just used from fb-helper and atomic-helper: */
>>>> -int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
>>>> - struct drm_plane_state *plane_state)
>>>> -{
>>>> - int ret;
>>>> -
>>>> - ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
>>>> - if (ret != 0)
>>>> - return ret;
>>>> -
>>>> - drm_atomic_set_fb_for_plane(plane_state, NULL);
>>>> - plane_state->crtc_x = 0;
>>>> - plane_state->crtc_y = 0;
>>>> - plane_state->crtc_w = 0;
>>>> - plane_state->crtc_h = 0;
>>>> - plane_state->src_x = 0;
>>>> - plane_state->src_y = 0;
>>>> - plane_state->src_w = 0;
>>>> - plane_state->src_h = 0;
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> -static int update_output_state(struct drm_atomic_state *state,
>>>> - struct drm_mode_set *set)
>>>> -{
>>>> - struct drm_device *dev = set->crtc->dev;
>>>> - struct drm_crtc *crtc;
>>>> - struct drm_crtc_state *new_crtc_state;
>>>> - struct drm_connector *connector;
>>>> - struct drm_connector_state *new_conn_state;
>>>> - int ret, i;
>>>> -
>>>> - ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
>>>> - state->acquire_ctx);
>>>> - if (ret)
>>>> - return ret;
>>>> -
>>>> - /* First disable all connectors on the target crtc. */
>>>> - ret = drm_atomic_add_affected_connectors(state, set->crtc);
>>>> - if (ret)
>>>> - return ret;
>>>> -
>>>> - for_each_new_connector_in_state(state, connector, new_conn_state, i) {
>>>> - if (new_conn_state->crtc == set->crtc) {
>>>> - ret = drm_atomic_set_crtc_for_connector(new_conn_state,
>>>> - NULL);
>>>> - if (ret)
>>>> - return ret;
>>>> -
>>>> - /* Make sure legacy setCrtc always re-trains */
>>>> - new_conn_state->link_status = DRM_LINK_STATUS_GOOD;
>>>> - }
>>>> - }
>>>> -
>>>> - /* Then set all connectors from set->connectors on the target crtc */
>>>> - for (i = 0; i < set->num_connectors; i++) {
>>>> - new_conn_state = drm_atomic_get_connector_state(state,
>>>> - set->connectors[i]);
>>>> - if (IS_ERR(new_conn_state))
>>>> - return PTR_ERR(new_conn_state);
>>>> -
>>>> - ret = drm_atomic_set_crtc_for_connector(new_conn_state,
>>>> - set->crtc);
>>>> - if (ret)
>>>> - return ret;
>>>> - }
>>>> -
>>>> - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>>>> - /* Don't update ->enable for the CRTC in the set_config request,
>>>> - * since a mismatch would indicate a bug in the upper layers.
>>>> - * The actual modeset code later on will catch any
>>>> - * inconsistencies here. */
>>>> - if (crtc == set->crtc)
>>>> - continue;
>>>> -
>>>> - if (!new_crtc_state->connector_mask) {
>>>> - ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
>>>> - NULL);
>>>> - if (ret < 0)
>>>> - return ret;
>>>> -
>>>> - new_crtc_state->active = false;
>>>> - }
>>>> - }
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> /**
>>>> * drm_atomic_helper_set_config - set a new config from userspace
>>>> * @set: mode set configuration
>>>> @@ -2964,81 +2875,6 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set,
>>>> }
>>>> EXPORT_SYMBOL(drm_atomic_helper_set_config);
>>>>
>>>> -/* just used from fb-helper and atomic-helper: */
>>>> -int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>>>> - struct drm_atomic_state *state)
>>>> -{
>>>> - struct drm_crtc_state *crtc_state;
>>>> - struct drm_plane_state *primary_state;
>>>> - struct drm_crtc *crtc = set->crtc;
>>>> - int hdisplay, vdisplay;
>>>> - int ret;
>>>> -
>>>> - crtc_state = drm_atomic_get_crtc_state(state, crtc);
>>>> - if (IS_ERR(crtc_state))
>>>> - return PTR_ERR(crtc_state);
>>>> -
>>>> - primary_state = drm_atomic_get_plane_state(state, crtc->primary);
>>>> - if (IS_ERR(primary_state))
>>>> - return PTR_ERR(primary_state);
>>>> -
>>>> - if (!set->mode) {
>>>> - WARN_ON(set->fb);
>>>> - WARN_ON(set->num_connectors);
>>>> -
>>>> - ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
>>>> - if (ret != 0)
>>>> - return ret;
>>>> -
>>>> - crtc_state->active = false;
>>>> -
>>>> - ret = drm_atomic_set_crtc_for_plane(primary_state, NULL);
>>>> - if (ret != 0)
>>>> - return ret;
>>>> -
>>>> - drm_atomic_set_fb_for_plane(primary_state, NULL);
>>>> -
>>>> - goto commit;
>>>> - }
>>>> -
>>>> - WARN_ON(!set->fb);
>>>> - WARN_ON(!set->num_connectors);
>>>> -
>>>> - ret = drm_atomic_set_mode_for_crtc(crtc_state, set->mode);
>>>> - if (ret != 0)
>>>> - return ret;
>>>> -
>>>> - crtc_state->active = true;
>>>> -
>>>> - ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);
>>>> - if (ret != 0)
>>>> - return ret;
>>>> -
>>>> - drm_mode_get_hv_timing(set->mode, &hdisplay, &vdisplay);
>>>> -
>>>> - drm_atomic_set_fb_for_plane(primary_state, set->fb);
>>>> - primary_state->crtc_x = 0;
>>>> - primary_state->crtc_y = 0;
>>>> - primary_state->crtc_w = hdisplay;
>>>> - primary_state->crtc_h = vdisplay;
>>>> - primary_state->src_x = set->x << 16;
>>>> - primary_state->src_y = set->y << 16;
>>>> - if (drm_rotation_90_or_270(primary_state->rotation)) {
>>>> - primary_state->src_w = vdisplay << 16;
>>>> - primary_state->src_h = hdisplay << 16;
>>>> - } else {
>>>> - primary_state->src_w = hdisplay << 16;
>>>> - primary_state->src_h = vdisplay << 16;
>>>> - }
>>>> -
>>>> -commit:
>>>> - ret = update_output_state(state, set);
>>>> - if (ret)
>>>> - return ret;
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> static int __drm_atomic_helper_disable_all(struct drm_device *dev,
>>>> struct drm_modeset_acquire_ctx *ctx,
>>>> bool clean_old_fbs)
>>>> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
>>>> index 216f2a9ee3d4..22a63a544ec7 100644
>>>> --- a/drivers/gpu/drm/drm_crtc_internal.h
>>>> +++ b/drivers/gpu/drm/drm_crtc_internal.h
>>>> @@ -207,6 +207,11 @@ struct drm_minor;
>>>> int drm_atomic_debugfs_init(struct drm_minor *minor);
>>>> #endif
>>>>
>>>> +int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
>>>> + struct drm_plane_state *plane_state);
>>>> +int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>>>> + struct drm_atomic_state *state);
>>>> +
>>>> void drm_atomic_print_state(const struct drm_atomic_state *state);
>>>>
>>>> /* drm_atomic_uapi.c */
>>>> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
>>>> index 58214be3bf3d..bf4e07141d81 100644
>>>> --- a/include/drm/drm_atomic_helper.h
>>>> +++ b/include/drm/drm_atomic_helper.h
>>>> @@ -117,12 +117,8 @@ int drm_atomic_helper_update_plane(struct drm_plane *plane,
>>>> struct drm_modeset_acquire_ctx *ctx);
>>>> int drm_atomic_helper_disable_plane(struct drm_plane *plane,
>>>> struct drm_modeset_acquire_ctx *ctx);
>>>> -int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
>>>> - struct drm_plane_state *plane_state);
>>>> int drm_atomic_helper_set_config(struct drm_mode_set *set,
>>>> struct drm_modeset_acquire_ctx *ctx);
>>>> -int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>>>> - struct drm_atomic_state *state);
>>>>
>>>> int drm_atomic_helper_disable_all(struct drm_device *dev,
>>>> struct drm_modeset_acquire_ctx *ctx);
>>>> --
>>>> 2.20.1
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>
>
>
More information about the dri-devel
mailing list