[ It seems a bit unfair to email you, but this is the patch that seems
to actually trigger the warning. Anyway, the intel-gfx list is also
CC'd.]
Hello Ander Conselvan de Oliveira,
The patch 304603f47a74: "drm/i915: Don't use staged config for VLV
cdclk calculations" from Apr 2, 2015, leads to the following Smatch
warning:
drivers/gpu/drm/i915/intel_display.c:12850 intel_crtc_set_config()
error: we previously assumed 'pipe_config' could be null (see line 12834)
drivers/gpu/drm/i915/intel_display.c
12808 /* Compute whether we need a full modeset, only an fb base update or no
12809 * change at all. In the future we might also check whether only the
12810 * mode changed, e.g. for LVDS where we only change the panel fitter in
12811 * such cases. */
12812 intel_set_config_compute_mode_changes(set, config);
This call can set "config->mode_changed" to true.
12813
12814 state = drm_atomic_state_alloc(dev);
12815 if (!state) {
12816 ret = -ENOMEM;
12817 goto out_config;
12818 }
12819
12820 state->acquire_ctx = dev->mode_config.acquire_ctx;
12821
12822 ret = intel_modeset_stage_output_state(dev, set, config, state);
12823 if (ret)
12824 goto fail;
12825
12826 pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
12827 state,
12828 &modeset_pipes,
12829 &prepare_pipes,
12830 &disable_pipes);
12831 if (IS_ERR(pipe_config)) {
12832 ret = PTR_ERR(pipe_config);
12833 goto fail;
12834 } else if (pipe_config) {
12835 if (pipe_config->has_audio !=
12836 to_intel_crtc(set->crtc)->config->has_audio)
12837 config->mode_changed = true;
^^^^^^^^^^^^^^^^^^^^^^^^^^^
Here we also set it to true if pipe_config is non-NULL.
12838
12839 /*
12840 * Note we have an issue here with infoframes: current code
12841 * only updates them on the full mode set path per hw
12842 * requirements. So here we should be checking for any
12843 * required changes and forcing a mode set.
12844 */
12845 }
12846
12847 intel_update_pipe_size(to_intel_crtc(set->crtc));
12848
12849 if (config->mode_changed) {
12850 ret = intel_set_mode_pipes(set->crtc, set->mode,
12851 set->x, set->y, set->fb, pipe_config,
^^^^^^^^^^^
Before this patch, then Smatch didn't print a warning because there were
some paths which didn't dereference pipe_config. The code here seems to
assume that if ->mode_changed is true then pipe_config is non-NULL but
my concern is that it's not considering the call to
intel_set_config_compute_mode_changes().
It's a weird thing that when you look at the code without considering
cross function analysis the code looks correct but when you do cross
function analysis it is very subtle.
12852 modeset_pipes, prepare_pipes,
12853 disable_pipes);
12854 } else if (config->fb_changed) {
12855 struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
12856 struct drm_plane *primary = set->crtc->primary;
12857 int vdisplay, hdisplay;
12858
regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
