[PATCH v2 4/5] drm/modes: Parse overscan properties

[Noralf Trønnes]

Den 17.04.2019 17.30, skrev Noralf Trønnes:
> 
> 
> Den 17.04.2019 16.07, skrev Maxime Ripard:
>> Hi Noralf,
>>
>> On Tue, Apr 16, 2019 at 04:52:20PM +0200, Noralf Trønnes wrote:
>>> Den 11.04.2019 15.22, skrev Maxime Ripard:
>>>> Properly configuring the overscan properties might be needed for the
>>>> initial setup of the framebuffer for display that still have overscan.
>>>> Let's allow for more properties on the kernel command line to setup each
>>>> margin.
>>>>
>>>> Signed-off-by: Maxime Ripard <maxime.ripard at bootlin.com>
>>>> ---
>>>>  drivers/gpu/drm/drm_fb_helper.c | 47 ++++++++++++++++++++++++++++++++++-
>>>>  drivers/gpu/drm/drm_modes.c     | 44 ++++++++++++++++++++++++++++++++-
>>>>  include/drm/drm_connector.h     |  1 +-
>>>>  3 files changed, 92 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>>>> index 8781897559b2..4e403fe1f451 100644
>>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>>> @@ -2567,6 +2567,51 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>>>>  	fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
>>>>  }
>>>>
>>>> +static void drm_setup_connector_margins(struct drm_connector *connector)
>>>> +{
>>>> +	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
>>>> +	struct drm_modeset_acquire_ctx ctx;
>>>> +	struct drm_atomic_state *state;
>>>> +	struct drm_device *dev = connector->dev;
>>>> +	int ret;
>>>> +
>>>> +	if (!drm_drv_uses_atomic_modeset(dev))
>>>> +		return;
>>>> +
>>>> +	drm_modeset_acquire_init(&ctx, 0);
>>>> +
>>>> +	state = drm_atomic_state_alloc(dev);
>>>> +	state->acquire_ctx = &ctx;
>>>> +
>>>> +retry:
>>>> +	drm_atomic_set_property(state, &connector->base,
>>>> +				dev->mode_config.tv_left_margin_property,
>>>> +				cmdline->overscan_left);
>>>> +
>>>> +	drm_atomic_set_property(state, &connector->base,
>>>> +				dev->mode_config.tv_right_margin_property,
>>>> +				cmdline->overscan_right);
>>>> +
>>>> +	drm_atomic_set_property(state, &connector->base,
>>>> +				dev->mode_config.tv_top_margin_property,
>>>> +				cmdline->overscan_top);
>>>> +
>>>> +	drm_atomic_set_property(state, &connector->base,
>>>> +				dev->mode_config.tv_bottom_margin_property,
>>>> +				cmdline->overscan_bottom);
>>>> +
>>>> +	ret = drm_atomic_commit(state);
>>>> +	if (ret == -EDEADLK) {
>>>> +		drm_atomic_state_clear(state);
>>>> +		drm_modeset_backoff(&ctx);
>>>> +		goto retry;
>>>> +	}
>>>> +
>>>> +	drm_atomic_state_put(state);
>>>> +	drm_modeset_drop_locks(&ctx);
>>>> +	drm_modeset_acquire_fini(&ctx);
>>>> +}
>>>> +
>>>
>>> Should we set these property values in drm_connector_init()?
>>> I assume that DRM userspace would want to use these values as well.
>>
>> I'm not sure, I've looked into it, and most drivers will create those
>> properties and attached *after* running drm_connector_init.
>>
>> If you're using drm_mode_create_tv_margin_properties, then the first
>> thing it does is to check whether that property has been created
>> already, so we're covered.
>>
>> For drm_connector_attach_tv_margins_properties though, this will force
>> overwrite the value.
>>
>> One thing I'm not really fond of either is that you would do this even
>> if the driver cannot support the margins at all.
>>
>> Maybe we can put this into drm_connector_attach_tv_margins_properties?
>>
> 
> That sounds like a good idea.
> It would leave out these which doesn't use that _attach function though:
> - i2c/ch7006_drv.c
> - i915/intel_tv.c
> 
> Not sure how to handle those. i915 uses the connector state to set the
> initial margins. Can we set the margins on the connector state in
> drm_connector_init() and use those as default values in
> drm_connector_attach_tv_margin_properties()?

Nevermind, that won't work as there's no state yet at that point if I'm
not mistaken.

> 
> Noralf.
> 
>> Maxime
>>
>> --
>> Maxime Ripard, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>