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

[Hans de Goede]

Hi,

On 12-11-2019 14:47, Daniel Vetter wrote:
> 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 ...

Ok, I will wait a bit for others to review this then. Note that Maxime has
reviewed patches 1-9, with one small remark on patch 9 which I'm still awaiting
an answer for. So most of the cmdline parsing stuff has been reviewed and
if you are ok with the non cmdline parsing changes...

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

Ok I will submit a v2 with this change.

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