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

[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()?

Noralf.

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>