Re: [Intel-gfx] [PATCH v3 03/13] drm/edid: clean up connector update error handling and debug logging

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Jun 22, 2022 at 01:59:17PM +0300, Jani Nikula wrote:
> Bail out on all errors, debug log all errors, and convert to drm device
> based debug logging.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 41 ++++++++++++++++++++++++++------------
>  1 file changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 62967db78139..e360e1a269f4 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6231,29 +6231,44 @@ static int _drm_connector_update_edid_property(struct drm_connector *connector,
>  
>  		if (old_edid) {
>  			if (!drm_edid_are_equal(drm_edid ? drm_edid->edid : NULL, old_edid)) {
> -				DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed.\n",
> -					      connector->base.id, connector->name);
> -
> -				connector->epoch_counter += 1;
> -				DRM_DEBUG_KMS("Updating change counter to %llu\n",
> -					      connector->epoch_counter);
> +				connector->epoch_counter++;
> +				drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed, epoch counter %llu\n",
> +					    connector->base.id, connector->name,
> +					    connector->epoch_counter);
>  			}
>  		}
>  	}
>  
> -	drm_object_property_set_value(&connector->base,
> -				      dev->mode_config.non_desktop_property,
> -				      connector->display_info.non_desktop);
> -
>  	ret = drm_property_replace_global_blob(dev,
>  					       &connector->edid_blob_ptr,
>  					       drm_edid ? drm_edid->size : 0,
>  					       drm_edid ? drm_edid->edid : NULL,
>  					       &connector->base,
>  					       dev->mode_config.edid_property);
> -	if (ret)
> -		return ret;
> -	return drm_connector_set_tile_property(connector);
> +	if (ret) {
> +		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID property update failed (%d)\n",
> +			    connector->base.id, connector->name, ret);
> +		goto out;
> +	}
> +
> +	ret = drm_object_property_set_value(&connector->base,
> +					    dev->mode_config.non_desktop_property,
> +					    connector->display_info.non_desktop);
> +	if (ret) {
> +		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Non-desktop property update failed (%d)\n",
> +			    connector->base.id, connector->name, ret);
> +		goto out;
> +	}
> +
> +	ret = drm_connector_set_tile_property(connector);
> +	if (ret) {
> +		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Tile property update failed (%d)\n",
> +			    connector->base.id, connector->name, ret);
> +		goto out;
> +	}
> +
> +out:

Could just return directly w/o the goto detour.
Or maybe this becomes useful later?

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	return ret;
>  }
>  
>  /**
> -- 
> 2.30.2

-- 
Ville Syrjälä
Intel