Commit e9c5fd26 authored by Paulo Zanoni's avatar Paulo Zanoni

drm/i915: set dev_priv->fbc.crtc before scheduling the enable work

This thing where we need to get the crtc either from the work
structure or the fbc structure itself is confusing and unnecessary.
Set fbc.crtc right when scheduling the enable work so we can always
use it.

The problem is not what gets passed and how to retrieve it. The
problem is that when we're in the other parts of the code we always
have to keep in mind that if FBC is already enabled we have to get the
CRTC from place A, if FBC is scheduled we have to get the CRTC from
place B, and if it's disabled there's no CRTC. Having a single place
to retrieve the CRTC from allows us to treat the "is enabled" and "is
scheduled" cases as the same case, reducing the mistake surface. I
guess I should add this to the commit message.

Besides the immediate advantages, this is also going to make one of
the next commits much simpler. And even later, when we introduce
enable/disable + activate/deactivate, this will be even simpler as
we'll set the CRTC at enable time. So all the
activate/deactivate/update code can just look at the single CRTC
variable regardless of the current state.

v2: Improve commit message (Chris).
v3: Rebase after changing the patch order.
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/
parent 90d5234f
...@@ -921,7 +921,6 @@ struct i915_fbc { ...@@ -921,7 +921,6 @@ struct i915_fbc {
struct intel_fbc_work { struct intel_fbc_work {
struct delayed_work work; struct delayed_work work;
struct intel_crtc *crtc;
struct drm_framebuffer *fb; struct drm_framebuffer *fb;
} *fbc_work; } *fbc_work;
......
...@@ -334,14 +334,13 @@ bool intel_fbc_enabled(struct drm_i915_private *dev_priv) ...@@ -334,14 +334,13 @@ bool intel_fbc_enabled(struct drm_i915_private *dev_priv)
return dev_priv->fbc.enabled; return dev_priv->fbc.enabled;
} }
static void intel_fbc_enable(struct intel_crtc *crtc, static void intel_fbc_enable(const struct drm_framebuffer *fb)
const struct drm_framebuffer *fb)
{ {
struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; struct drm_i915_private *dev_priv = fb->dev->dev_private;
struct intel_crtc *crtc = dev_priv->fbc.crtc;
dev_priv->fbc.enable_fbc(crtc); dev_priv->fbc.enable_fbc(crtc);
dev_priv->fbc.crtc = crtc;
dev_priv->fbc.fb_id = fb->base.id; dev_priv->fbc.fb_id = fb->base.id;
dev_priv->fbc.y = crtc->base.y; dev_priv->fbc.y = crtc->base.y;
} }
...@@ -351,8 +350,8 @@ static void intel_fbc_work_fn(struct work_struct *__work) ...@@ -351,8 +350,8 @@ static void intel_fbc_work_fn(struct work_struct *__work)
struct intel_fbc_work *work = struct intel_fbc_work *work =
container_of(to_delayed_work(__work), container_of(to_delayed_work(__work),
struct intel_fbc_work, work); struct intel_fbc_work, work);
struct drm_i915_private *dev_priv = work->crtc->base.dev->dev_private; struct drm_i915_private *dev_priv = work->fb->dev->dev_private;
struct drm_framebuffer *crtc_fb = work->crtc->base.primary->fb; struct drm_framebuffer *crtc_fb = dev_priv->fbc.crtc->base.primary->fb;
mutex_lock(&dev_priv->fbc.lock); mutex_lock(&dev_priv->fbc.lock);
if (work == dev_priv->fbc.fbc_work) { if (work == dev_priv->fbc.fbc_work) {
...@@ -360,7 +359,7 @@ static void intel_fbc_work_fn(struct work_struct *__work) ...@@ -360,7 +359,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
* the prior work. * the prior work.
*/ */
if (crtc_fb == work->fb) if (crtc_fb == work->fb)
intel_fbc_enable(work->crtc, work->fb); intel_fbc_enable(work->fb);
dev_priv->fbc.fbc_work = NULL; dev_priv->fbc.fbc_work = NULL;
} }
...@@ -400,15 +399,15 @@ static void intel_fbc_schedule_enable(struct intel_crtc *crtc) ...@@ -400,15 +399,15 @@ static void intel_fbc_schedule_enable(struct intel_crtc *crtc)
WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
intel_fbc_cancel_work(dev_priv); intel_fbc_cancel_work(dev_priv);
dev_priv->fbc.crtc = crtc;
work = kzalloc(sizeof(*work), GFP_KERNEL); work = kzalloc(sizeof(*work), GFP_KERNEL);
if (work == NULL) { if (work == NULL) {
DRM_ERROR("Failed to allocate FBC work structure\n"); DRM_ERROR("Failed to allocate FBC work structure\n");
intel_fbc_enable(crtc, crtc->base.primary->fb); intel_fbc_enable(crtc->base.primary->fb);
return; return;
} }
work->crtc = crtc;
work->fb = crtc->base.primary->fb; work->fb = crtc->base.primary->fb;
INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn); INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn);
...@@ -984,11 +983,8 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv, ...@@ -984,11 +983,8 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
mutex_lock(&dev_priv->fbc.lock); mutex_lock(&dev_priv->fbc.lock);
if (dev_priv->fbc.enabled) if (dev_priv->fbc.enabled || dev_priv->fbc.fbc_work)
fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe); fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
else if (dev_priv->fbc.fbc_work)
fbc_bits = INTEL_FRONTBUFFER_PRIMARY(
dev_priv->fbc.fbc_work->crtc->pipe);
else else
fbc_bits = dev_priv->fbc.possible_framebuffer_bits; fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment