Re: [RFC][PATCH 06/11] drm/edid: Don't call drm_add_display_info() with an invalid EDID

From: Daniel Vetter <daniel@ffwll.ch>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [RFC][PATCH 06/11] drm/edid: Don't call drm_add_display_info() with an invalid EDID
Date: Tue, 6 Mar 2018 10:45:22 +0100	[thread overview]
Message-ID: <20180306094522.GU22212@phenom.ffwll.local> (raw)
In-Reply-To: <20180227125700.6527-7-ville.syrjala@linux.intel.com>

On Tue, Feb 27, 2018 at 02:56:55PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> drm_mode_connector_update_edid_property() will call
> drm_add_display_info() if an invalid edid is passed in.
> This differs from the behaviour of drm_add_edid_modes() which
> doesn't try to populate the display info from an invalid edid.
> Adjust drm_mode_connector_update_edid_property() to match that
> behaviour.
> 
> Unfortunately we have to pass the edid as non-const to
> drm_mode_connector_update_edid_property() because drm_edid_is_valid()
> may need to modify it :( Would be nice to one day fix up the EDID code
> to not do crazy things like that.

I guess we could put the EDID validation into the EDID reading helpers and
just pray that any fixed EDID (from vbt or wherever) is actually valid?

Meanwhile this makes sense to me.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
> Cc: Keith Packard <keithp@keithp.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 4 ++--
>  include/drm/drm_connector.h     | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index ddd7d978f462..d8c3ef4f17da 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1368,7 +1368,7 @@ EXPORT_SYMBOL(drm_mode_connector_set_tile_property);
>   * Zero on success, negative errno on failure.
>   */
>  int drm_mode_connector_update_edid_property(struct drm_connector *connector,
> -					    const struct edid *edid)
> +					    struct edid *edid)
>  {
>  	struct drm_device *dev = connector->dev;
>  	size_t size = 0;
> @@ -1390,7 +1390,7 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector,
>  	 * ordering of calls.
>  	 */
>  	drm_reset_display_info(connector);
> -	if (edid)
> +	if (edid && drm_edid_is_valid(edid))
>  		drm_add_display_info(connector, edid);
>  
>  	drm_object_property_set_value(&connector->base,
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index aad3258facf2..8815ef1ce429 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1119,7 +1119,7 @@ int drm_mode_connector_set_path_property(struct drm_connector *connector,
>  					 const char *path);
>  int drm_mode_connector_set_tile_property(struct drm_connector *connector);
>  int drm_mode_connector_update_edid_property(struct drm_connector *connector,
> -					    const struct edid *edid);
> +					    struct edid *edid);
>  void drm_mode_connector_set_link_status_property(struct drm_connector *connector,
>  						 uint64_t link_status);
>  int drm_connector_init_panel_orientation_property(
> -- 
> 2.13.6
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx