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

Maxime Ripard
2019-04-17T14:07:34Z

[-- Attachment #1.1: Type: text/plain, Size: 3333 bytes --]

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@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?

Maxime

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]