On Tue, Nov 12, 2019 at 2:39 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi, > > On 12-11-2019 14:32, Daniel Vetter wrote: > > On Tue, Nov 12, 2019 at 11:43 AM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx> > >>> > >>> 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@xxxxxxxxxxxxxxxxxxxxx > >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >>> > >> > > > > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
