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.rip...@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 > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel