Op 05-04-17 om 12:06 schreef Daniel Vetter:
> On Wed, Apr 05, 2017 at 10:41:24AM +0200, Maarten Lankhorst wrote:
>> The connector atomic check function may be called multiple times,
>> or not at all. It's mostly useful for implementing properties but if you
>> call check_modeset twice it can be used for other modeset related checks
>> as well.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> ---
>> drivers/gpu/drm/drm_atomic_helper.c | 25 +++++++++++++++++----
>> include/drm/drm_modeset_helper_vtables.h | 37 ++++++++++++++++++++++++++++++++
>> 2 files changed, 58 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index c3994b4d5f32..d550e79e8347 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -459,10 +459,20 @@ mode_fixup(struct drm_atomic_state *state)
>> *
>> * Check the state object to see if the requested state is physically possible.
>> * This does all the crtc and connector related computations for an atomic
>> - * update and adds any additional connectors needed for full modesets and calls
>> - * down into &drm_crtc_helper_funcs.mode_fixup and
>> - * &drm_encoder_helper_funcs.mode_fixup or
>> - * &drm_encoder_helper_funcs.atomic_check functions of the driver backend.
>> + * update and adds any additional connectors needed for full modesets. It calls
>> + * the various per-object callbacks in the follow order:
>> + *
>> + * 1. &drm_connector_helper_funcs.atomic_best_encoder for determining the new encoder.
>> + * 2. &drm_connector_helper_funcs.atomic_check to validate the connector state.
>> + * 3. If it's determined a modeset is needed then all connectors on the affected crtc
>> + * crtc are added.
>> + * 4. &drm_bridge_funcs.mode_fixup is called on all encoder bridges.
>> + * 5. &drm_encoder_helper_funcs.atomic_check is called to validate any encoder state.
>> + * This function is only called when the encoder will be part of a configured crtc,
>> + * it must not be used for implementing connector property validation.
>> + * If this function is NULL, &drm_atomic_encoder_helper_funcs.mode_fixup is called
>> + * instead.
>> + * 6. &drm_crtc_helper_funcs.mode_fixup is called last, to fix up the mode with crtc constraints.
> Line too I think. And maybe insert empty lines between each item, to make
> them stand out more. Shouldn't affect rendering, but better to recheck.
>
>> *
>> * &drm_crtc_state.mode_changed is set when the input mode is changed.
>> * &drm_crtc_state.connectors_changed is set when a connector is added or
>> @@ -522,6 +532,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>> return ret;
>>
>> for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) {
>> + const struct drm_connector_helper_funcs *funcs = connector->helper_private;
>> +
>> /*
>> * This only sets crtc->connectors_changed for routing changes,
>> * drivers must set crtc->connectors_changed themselves when
>> @@ -539,6 +551,11 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>> new_connector_state->link_status)
>> new_crtc_state->connectors_changed = true;
>> }
>> +
>> + if (funcs->atomic_check)
>> + ret = funcs->atomic_check(connector, new_connector_state);
>> + if (ret)
>> + return ret;
>> }
>>
>> /*
>> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
>> index b304950b402d..7b5dd909f189 100644
>> --- a/include/drm/drm_modeset_helper_vtables.h
>> +++ b/include/drm/drm_modeset_helper_vtables.h
>> @@ -860,6 +860,43 @@ struct drm_connector_helper_funcs {
>> */
>> struct drm_encoder *(*atomic_best_encoder)(struct drm_connector *connector,
>> struct drm_connector_state *connector_state);
>> +
>> + /**
>> + * @atomic_check:
>> + *
>> + * Drivers should check connector properties in this
>> + * hook. This function is called from &drm_atomic_helper_check_modeset.
>> + *
>> + * This function may not be called at all on a modeset or connector
>> + * change, so it should only be used to validate properties.
>> + * If it's required to validate all connectors from there, call
>> + * &drm_atomic_helper_check_modeset twice. This will result in
>> + * atomic_check possibly being called twice as well, which the callback
>> + * should be made to handle correctly.
>> + *
>> + * This function is also allowed to inspect any other object's state and
>> + * can add more state objects to the atomic commit if needed. Care must
>> + * be taken though to ensure that state check and compute functions for
>> + * these added states are all called, and derived state in other objects
>> + * all updated. Again the recommendation is to just call check helpers
>> + * until a maximal configuration is reached.
> I think it'd be good to highlight setting connectors_changed from this
> callback:
>
> "If this callback determines that the property change requires a full
> modeset, it can set &drm_crtc_state->connectors_changed to steer the
> control flow in drm_atomic_helper_check_modeset()."
>
> With those nits addressed, Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
>
> Would be good to get DK to r-b the patch too before merging, just for
> practice and coordination.
>
> Cheers, Daniel
Could you review?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
