On Thu, Feb 25, 2021 at 10:34 PM 'Nick Desaulniers' via Clang Built Linux <clang-built-linux@xxxxxxxxxxxxxxxx> wrote: > return parse_edid_cea(aconnector, edid_ext, EDID_LENGTH, vsdb_info) ? i : -ENODEV; > > would suffice, but the patch is still fine as is. Right, I did not want to change more than necessary here, and the original code already had the extra assignment instead of returning the value. > > @@ -9857,8 +9857,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector, > > } > > } > > } else if (edid && amdgpu_dm_connector->dc_sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A) { > > - hdmi_valid_vsdb_found = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info); > > - if (hdmi_valid_vsdb_found && vsdb_info.freesync_supported) { > > + i = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info); > > + if (i >= 0 && vsdb_info.freesync_supported) { > > reusing `i` here is safe, for now, but reuse of variables like this in > separate branches like this might not get noticed if the function is > amended in the future. > > > timing = &edid->detailed_timings[i]; > > data = &timing->data.other_data; The entire point of the patch is that 'i' is in fact used in the following line, but was lacking an intialization. Arnd _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel