Skip to content

Commit

Permalink
drm/i915: Introduce better global state handling
Browse files Browse the repository at this point in the history
Our current global state handling is pretty ad-hoc. Let's try to
make it better by imitating the standard drm core private object
approach.

The reason why we don't want to directly use the private objects
is locking; Each private object has its own lock so if we
introduce any global private objects we get serialized by that
single lock across all pipes. The global state apporoach instead
uses a read/write lock type of approach where each individual
crtc lock counts as a read lock, and grabbing all the crtc locks
allows one write access.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200120174728.21095-15-ville.syrjala@linux.intel.com
Reviewed-by: Imre Deak <imre.deak@intel.com>
  • Loading branch information
Ville Syrjälä committed Jan 31, 2020
1 parent 5f34299 commit 0ef1905
Show file tree
Hide file tree
Showing 10 changed files with 342 additions and 12 deletions.
1 change: 1 addition & 0 deletions drivers/gpu/drm/i915/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ i915-y += \
display/intel_fbc.o \
display/intel_fifo_underrun.o \
display/intel_frontbuffer.o \
display/intel_global_state.o \
display/intel_hdcp.o \
display/intel_hotplug.o \
display/intel_lpe_audio.o \
Expand Down
7 changes: 5 additions & 2 deletions drivers/gpu/drm/i915/display/intel_atomic.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "intel_atomic.h"
#include "intel_cdclk.h"
#include "intel_display_types.h"
#include "intel_global_state.h"
#include "intel_hdcp.h"
#include "intel_psr.h"
#include "intel_sprite.h"
Expand Down Expand Up @@ -502,6 +503,7 @@ void intel_atomic_state_free(struct drm_atomic_state *_state)
struct intel_atomic_state *state = to_intel_atomic_state(_state);

drm_atomic_state_default_release(&state->base);
kfree(state->global_objs);

i915_sw_fence_fini(&state->commit_ready);

Expand All @@ -513,6 +515,7 @@ void intel_atomic_state_clear(struct drm_atomic_state *s)
struct intel_atomic_state *state = to_intel_atomic_state(s);

drm_atomic_state_default_clear(&state->base);
intel_atomic_clear_global_state(state);

state->dpll_set = state->modeset = false;
state->global_state_changed = false;
Expand All @@ -532,7 +535,7 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
return to_intel_crtc_state(crtc_state);
}

int intel_atomic_lock_global_state(struct intel_atomic_state *state)
int _intel_atomic_lock_global_state(struct intel_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
struct intel_crtc *crtc;
Expand All @@ -551,7 +554,7 @@ int intel_atomic_lock_global_state(struct intel_atomic_state *state)
return 0;
}

int intel_atomic_serialize_global_state(struct intel_atomic_state *state)
int _intel_atomic_serialize_global_state(struct intel_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
struct intel_crtc *crtc;
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/i915/display/intel_atomic.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
struct intel_crtc *intel_crtc,
struct intel_crtc_state *crtc_state);

int intel_atomic_lock_global_state(struct intel_atomic_state *state);
int _intel_atomic_lock_global_state(struct intel_atomic_state *state);

int intel_atomic_serialize_global_state(struct intel_atomic_state *state);
int _intel_atomic_serialize_global_state(struct intel_atomic_state *state);

#endif /* __INTEL_ATOMIC_H__ */
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/display/intel_audio.c
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
enable ? 2 * 96000 : 0;

/* Protects dev_priv->cdclk.force_min_cdclk */
ret = intel_atomic_lock_global_state(to_intel_atomic_state(state));
ret = _intel_atomic_lock_global_state(to_intel_atomic_state(state));
if (!ret)
ret = drm_atomic_commit(state);

Expand Down
8 changes: 4 additions & 4 deletions drivers/gpu/drm/i915/display/intel_cdclk.c
Original file line number Diff line number Diff line change
Expand Up @@ -2080,7 +2080,7 @@ static int intel_compute_min_cdclk(struct intel_atomic_state *state)

cdclk_state->min_cdclk[i] = min_cdclk;

ret = intel_atomic_lock_global_state(state);
ret = _intel_atomic_lock_global_state(state);
if (ret)
return ret;
}
Expand Down Expand Up @@ -2128,7 +2128,7 @@ static int bxt_compute_min_voltage_level(struct intel_atomic_state *state)

cdclk_state->min_voltage_level[i] = min_voltage_level;

ret = intel_atomic_lock_global_state(state);
ret = _intel_atomic_lock_global_state(state);
if (ret)
return ret;
}
Expand Down Expand Up @@ -2403,12 +2403,12 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
* Also serialize commits across all crtcs
* if the actual hw needs to be poked.
*/
ret = intel_atomic_serialize_global_state(state);
ret = _intel_atomic_serialize_global_state(state);
if (ret)
return ret;
} else if (intel_cdclk_changed(&old_cdclk_state->logical,
&new_cdclk_state->logical)) {
ret = intel_atomic_lock_global_state(state);
ret = _intel_atomic_lock_global_state(state);
if (ret)
return ret;
} else {
Expand Down
15 changes: 12 additions & 3 deletions drivers/gpu/drm/i915/display/intel_display.c
Original file line number Diff line number Diff line change
Expand Up @@ -14562,7 +14562,7 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
}

if (state->active_pipe_changes) {
ret = intel_atomic_lock_global_state(state);
ret = _intel_atomic_lock_global_state(state);
if (ret)
return ret;
}
Expand Down Expand Up @@ -15842,6 +15842,8 @@ static int intel_atomic_commit(struct drm_device *dev,
ret = drm_atomic_helper_setup_commit(&state->base, nonblock);
if (!ret)
ret = drm_atomic_helper_swap_state(&state->base, true);
if (!ret)
intel_atomic_swap_global_state(state);

if (ret) {
i915_sw_fence_commit(&state->commit_ready);
Expand Down Expand Up @@ -17759,6 +17761,7 @@ static void intel_mode_config_init(struct drm_i915_private *i915)
struct drm_mode_config *mode_config = &i915->drm.mode_config;

drm_mode_config_init(&i915->drm);
INIT_LIST_HEAD(&i915->global_obj_list);

mode_config->min_width = 0;
mode_config->min_height = 0;
Expand Down Expand Up @@ -17800,6 +17803,12 @@ static void intel_mode_config_init(struct drm_i915_private *i915)
}
}

static void intel_mode_config_cleanup(struct drm_i915_private *i915)
{
intel_atomic_global_obj_cleanup(i915);
drm_mode_config_cleanup(&i915->drm);
}

int intel_modeset_init(struct drm_i915_private *i915)
{
struct drm_device *dev = &i915->drm;
Expand Down Expand Up @@ -17839,7 +17848,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
for_each_pipe(i915, pipe) {
ret = intel_crtc_init(i915, pipe);
if (ret) {
drm_mode_config_cleanup(dev);
intel_mode_config_cleanup(i915);
return ret;
}
}
Expand Down Expand Up @@ -18807,7 +18816,7 @@ void intel_modeset_driver_remove(struct drm_i915_private *i915)

intel_hdcp_component_fini(i915);

drm_mode_config_cleanup(&i915->drm);
intel_mode_config_cleanup(i915);

intel_overlay_cleanup(i915);

Expand Down
4 changes: 4 additions & 0 deletions drivers/gpu/drm/i915/display/intel_display_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "intel_de.h"

struct drm_printer;
struct __intel_global_objs_state;

/*
* Display related stuff
Expand Down Expand Up @@ -462,6 +463,9 @@ struct intel_atomic_state {

intel_wakeref_t wakeref;

struct __intel_global_objs_state *global_objs;
int num_global_objs;

struct intel_cdclk_state cdclk_state;

bool dpll_set, modeset;
Expand Down
Loading

0 comments on commit 0ef1905

Please sign in to comment.