Den 27.03.2019 14.55, skrev Daniel Vetter: > On Wed, Mar 27, 2019 at 2:42 PM Noralf Trønnes <noralf@xxxxxxxxxxx> 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@xxxxxxxxxxx> >>> >>> Triggers a bit my midlayer ocd, but I can't come up with a better idea >>> either. >>> >>> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> >>> >> >> 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@xxxxxxxxxxxxxxxxxxxxx >>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >>> > > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
