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

[Hans de Goede]

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 ?

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.

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