[Intel-gfx] [PATCH v3 3/3] drm: clean cached display info

[Jose Abreu]

Hi Shashank,


On 29-12-2016 05:53, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 12/27/2016 3:07 PM, Daniel Vetter wrote:
>> On Thu, Dec 22, 2016 at 10:21:25AM +0000, Jose Abreu wrote:
>>> Hi Shashank,
>>>
>>>
>>> On 21-12-2016 15:29, Shashank Sharma wrote:
>>>> This patch adds a small helper function, which clears the
>>>> cached
>>>> information about a hot-pluggable display, from connector.
>>>> On event
>>>> This will run on event of a hot-unplug, keeping the
>>>> connector's display
>>>> info up-to-date, avoiding any errors due to invalid cached
>>>> data.
>>>>
>>>> Cc: Jose Abreu <joabreu at synopsys.com>
>>>>
>>>> Suggested-by: Jose Abreu <joabreu at synopsys.com>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>>>> ---
>>>>   drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++
>>>>   1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_probe_helper.c
>>>> b/drivers/gpu/drm/drm_probe_helper.c
>>>> index 7cff91e..9e97b45 100644
>>>> --- a/drivers/gpu/drm/drm_probe_helper.c
>>>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>>>> @@ -164,6 +164,18 @@ void
>>>> drm_kms_helper_poll_enable_locked(struct drm_device *dev)
>>>>   }
>>>>     /**
>>>> + * drm_helper_clear_display_info - clean cached display
>>>> information for
>>>> + * hot pluggable displays, on event of hot-unplug
>>>> + * @connector: connector under event
>>>> + */
>>>> +void drm_helper_clear_display_info(struct drm_connector
>>>> *connector)
>>>> +{
>>>> +    struct drm_display_info *info = &connector->display_info;
>>>> +
>>>> +    memset(info, 0, sizeof(*info));
>>>> +}
>>>> +
>>>> +/**
>>>>    * drm_helper_probe_single_connector_modes - get complete
>>>> set of display modes
>>>>    * @connector: connector to probe
>>>>    * @maxX: max width for modes
>>>> @@ -288,6 +300,14 @@ int
>>>> drm_helper_probe_single_connector_modes(struct drm_connector
>>>> *connector,
>>>>           DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n",
>>>>               connector->base.id, connector->name);
>>>>           drm_mode_connector_update_edid_property(connector,
>>>> NULL);
>>>> +
>>>> +        /*
>>>> +         * Connector status change to disconnected, time to
>>>> clean
>>>> +         * cached display information
>>>> +         */
>>>> +        if (connector->status ==
>>>> connector_status_disconnected)
>>>> +            drm_helper_clear_display_info(connector);
>>>> +
>>> I don't know if this is the right place to do this because it is
>>> a helper and I don't know if it is used by all the drivers. We
>>> may need something more general that is always called when
>>> probing modes, or force drivers that don't use the helper to use
>>> the drm_helper_clear_display_info function. As I told you
>>> before,
>>> I'm new to dri-devel so we need more comments.
>> Seems reasonable to me, since afaik all drivers do use the
>> probe helpers.
>> -Daniel
> This was my understanding too. Jose, you think there would be
> any drivers who dont use this probe ?
> - Shashank

I found only one driver that don't use this helper: vmwgfx. But,
this driver does not seem to use EDID fields, it has a list of
preferred video modes and manually adds these modes.

So, I think it is safe to add this in the helper as long as
future drivers that use EDID use this helper also. Maybe a small
comment about this should be added in the helper declaration.

Best regards,
Jose Miguel Abreu

>>> Best regards,
>>> Jose Miguel Abreu
>>>
>>>>           verbose_prune = false;
>>>>           goto prune;
>>>>       }
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_intel-2Dgfx&d=DgIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=1G5dnBp7Y6VEifpEnDT2wKFoDRBXnxGXAnA-4883H74&s=y1M2ce128zpR_lBDPSgS_JGm-HoPIJjneK2s3tkrvyo&e=
>>
>