Re: [PATCH v6 02/12] drm/atomic: Add support for custom scaling mode properties, v2

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 02-05-17 om 11:44 schreef Daniel Vetter:
> On Mon, May 01, 2017 at 03:37:54PM +0200, Maarten Lankhorst wrote:
>> Some connectors may not allow all scaling mode properties, this function will allow
>> creating the scaling mode property with only the supported subset. It also wires up
>> this state for atomic.
>>
>> This will make it possible to convert i915 connectors to atomic.
>>
>> Changes since v1:
>>  - Add DRM_MODE_PROP_ENUM flag to drm_property_create
>>  - Use the correct index in drm_property_add_enum.
>>  - Add DocBook for function (Sean Paul).
>>  - Warn if less than 2 valid scaling modes are passed.
>>  - Remove level of indent. (Sean Paul)
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/drm_atomic.c    |  4 +++
>>  drivers/gpu/drm/drm_connector.c | 58 +++++++++++++++++++++++++++++++++++++++++
>>  include/drm/drm_connector.h     | 10 +++++++
>>  3 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 77bb36e956db..c7f91dcebbe9 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -1125,6 +1125,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
>>  			state->link_status = val;
>>  	} else if (property == config->aspect_ratio_property) {
>>  		state->picture_aspect_ratio = val;
>> +	} else if (property == connector->scaling_mode_property) {
>> +		state->scaling_mode = val;
> Can't we still handle mode_config->scaling_mode_property as fallback?
> Seems a lot more consistent to me ...
>
>>  	} else if (connector->funcs->atomic_set_property) {
>>  		return connector->funcs->atomic_set_property(connector,
>>  				state, property, val);
>> @@ -1203,6 +1205,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>>  		*val = state->link_status;
>>  	} else if (property == config->aspect_ratio_property) {
>>  		*val = state->picture_aspect_ratio;
>> +	} else if (property == connector->scaling_mode_property) {
>> +		*val = state->scaling_mode;
>>  	} else if (connector->funcs->atomic_get_property) {
>>  		return connector->funcs->atomic_get_property(connector,
>>  				state, property, val);
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 9f847615ac74..b3912f2e48c6 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -961,6 +961,64 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev)
>>  EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>>  
>>  /**
>> + * drm_mode_connector_attach_scaling_mode_property - attach atomic scaling mode property
>> + * @connector: connector to attach scaling mode property on.
>> + * @scaling_mode_mask: or'ed mask of BIT(DRM_MODE_SCALE_\*).
>> + *
>> + * This is used to add support for scaling mode to atomic drivers.
>> + * The scaling mode will be set to &struct drm_connector_state->picture_aspect_ratio
> s/->/./ to get a real link
>
>> + * and can be used from &struct drm_connector_helper_funcs->atomic_check for validation.
> Same here, plus needs &.
>
> Please check the html output when typing docs ...
>
> Also please link to drm_mode_create_scaling_mode_property() and from the
> kerneldoc of that to this one here.
Ah right, thanks.
>
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int drm_mode_connector_attach_scaling_mode_property(struct drm_connector *connector,
>> +						    u32 scaling_mode_mask)
> Usual prefix is just drm_connector_ (yes I know we're not consistent here,
> yet).
>
> With those nits: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
Will rename. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx