Re: [RFC][PATCH 05/11] drm/edid: Clear display info fully

["Sharma, Shashank" <shashank.sharma@intel.com>]

Regards

Shashank


On 2/27/2018 6:26 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that we have split the display info into static and dynamic parts,
> we can just zero out the entire dynamic part with memset(). Previously
> we were just clearing parts of it, leaving stale data in other parts
> (eg. HDMI SCDC capabilities).
>
> Also when the edid is NULL drm_add_edid_modes() bails out early skipping
> the call to drm_add_display_info(). Thus we would again leave stale
> data behind. To avoid that let's clear out the display info at the
> very start of drm_add_edid_modes().
>
> Cc: Keith Packard <keithp@keithp.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/drm_connector.c |  3 +--
>   drivers/gpu/drm/drm_edid.c      | 23 +++--------------------
>   2 files changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index d73e97ed7dff..ddd7d978f462 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1389,10 +1389,9 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector,
>   	 * duplicate it rather than attempt to ensure some arbitrary
>   	 * ordering of calls.
>   	 */
> +	drm_reset_display_info(connector);
>   	if (edid)
>   		drm_add_display_info(connector, edid);
> -	else
> -		drm_reset_display_info(connector);
>   
>   	drm_object_property_set_value(&connector->base,
>   				      dev->mode_config.non_desktop_property,
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 788fee4b4bf9..78c1f37be3db 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4443,37 +4443,18 @@ drm_reset_display_info(struct drm_connector *connector)
>   {
>   	struct drm_display_info *info = &connector->display_info;
>   
> -	info->width_mm = 0;
> -	info->height_mm = 0;
> -
> -	info->bpc = 0;
> -	info->color_formats = 0;
> -	info->cea_rev = 0;
> -	info->max_tmds_clock = 0;
> -	info->dvi_dual = false;
> -	info->has_hdmi_infoframe = false;
> -
> -	info->non_desktop = 0;
> +	memset(info, 0, sizeof(*info));
>   }
>   EXPORT_SYMBOL_GPL(drm_reset_display_info);
>   
>   u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid)
>   {
>   	struct drm_display_info *info = &connector->display_info;
> -
>   	u32 quirks = edid_get_quirks(edid);
>   
>   	info->width_mm = edid->width_cm * 10;
>   	info->height_mm = edid->height_cm * 10;
>   
> -	/* driver figures it out in this case */
> -	info->bpc = 0;
> -	info->color_formats = 0;
> -	info->cea_rev = 0;
> -	info->max_tmds_clock = 0;
> -	info->dvi_dual = false;
> -	info->has_hdmi_infoframe = false;
> -
>   	info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
>   
>   	DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
> @@ -4684,6 +4665,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>   	int num_modes = 0;
>   	u32 quirks;
>   
> +	drm_reset_display_info(connector);
How about if we move the call to drm_reset_display_info() within 
drm_add_display_info() ?
something like:
drm_add_display_info()
{
     struct drm_display_info *info = &connector->display_info;
     u32 quirks = edid_get_quirks(edid);

     drm_reset_display_info();

     info->width_mm = edid->width_cm * 10;
     info->height_mm = edid->height_cm * 10;
     info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);

     ......
}

- Shashank
> +
>   	if (edid == NULL) {
>   		clear_eld(connector);
>   		return 0;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx