[PATCH 12/12] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member

[Daniel Vetter]

On Tue, Nov 12, 2019 at 2:39 PM Hans de Goede <hdegoede at redhat.com> wrote:
>
> Hi,
>
> On 12-11-2019 14:32, Daniel Vetter wrote:
> > On Tue, Nov 12, 2019 at 11:43 AM Hans de Goede <hdegoede at redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 12-11-2019 10:47, Daniel Vetter wrote:
> >>> On Sun, Nov 10, 2019 at 04:41:01PM +0100, Hans de Goede wrote:
> >>>> If the new video=... panel_orientation option is set for a connector, honor
> >>>> it and setup a matching "panel orientation" property on the connector.
> >>>>
> >>>> BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
> >>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> >>>
> >>> Don't we need one more patch here to create the panel orientation property
> >>> if the driver doesn't do it? Otherwise this won't happen, and userspace
> >>> can't look at it (only the internal fbdev stuff, which has it's own
> >>> limitations wrt rotation).
> >>
> >> That is what patch 11/12 is for, that patch renames drm_connector_init_panel_orientation
> >> to drm_set_panel_orientation and makes it both set, init and attach the property
> >> (unless called with DRM_MODE_PANEL_ORIENTATION_UNKNOWN in which case it is a no-op).
> >>
> >> Patch 11/12 also makes drm_set_panel_orientation do the set only once, which is why
> >> the orientation to set is now passed instead of stored directly in the connector,
> >> so that a second set call (panel_orientation of the connector already !=
> >> DRM_MODE_PANEL_ORIENTATION_UNKNOWN) can be skipped, so that the cmdline overrides
> >> driver detecion code for this.
> >
> > Oh, that's what I get for not reading the entire series ... Only risk
> > with that is that drivers set this property after driver loading is
> > done - we can't attach/create properties after drm_dev_register. But
> > I've added the corresponding WARN_ON to these, so we should be all
> > fine I think. So looks all good to me.
>
> Can I take that as your Acked-by for this patch and perhaps also for 11/12 ?

Uh I didn't really read the details, ack feels a bit thin to land this ...

> Also what about your remarks to 10/12?  I'm happy to do a v2 with a memset,
> as said my main reason for dropping the specified=false in the early path
> is that we never init bpp_specified or refresh_specified explicitly to false
> I'm all for being explicit about that, but then lets just zero out the entire
> passed in drm_cmdline_mode struct.

Hm yeah, clearing it all might be a good idea.
-Daniel
>
> Regards,
>
> Hans
>
>
>
> >>>> ---
> >>>>    drivers/gpu/drm/drm_connector.c | 7 +++++++
> >>>>    1 file changed, 7 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> >>>> index 40a985c411a6..591914f01cd4 100644
> >>>> --- a/drivers/gpu/drm/drm_connector.c
> >>>> +++ b/drivers/gpu/drm/drm_connector.c
> >>>> @@ -140,6 +140,13 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
> >>>>               connector->force = mode->force;
> >>>>       }
> >>>>
> >>>> +    if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
> >>>> +            DRM_INFO("setting connector %s panel_orientation to %d\n",
> >>>> +                     connector->name, mode->panel_orientation);
> >>>> +            drm_connector_set_panel_orientation(connector,
> >>>> +                                                mode->panel_orientation);
> >>>> +    }
> >>>> +
> >>>>       DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
> >>>>                     connector->name, mode->name,
> >>>>                     mode->xres, mode->yres,
> >>>> --
> >>>> 2.23.0
> >>>>
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel at lists.freedesktop.org
> >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>
> >
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch