Em Qui, 2016-01-21 às 13:48 +0100, Maarten Lankhorst escreveu:
> Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> > The early return inside __intel_fbc_update does not completely
> > check
> > all the parameters that affect the FBC register values. For
> > example,
> > we currently lack looking at crtc->adjusted_y (for the fence Y
> > offset)
> > and all the parameters that affect the CFB size (for i8xx).
> >
> > Instead of just adding the missing parameters to the check and
> > hoping
> > that any changes to the fbc_activate functions also come with a
> > matching change to the __intel_fbc_update check, introduce a new
> > structure where we store these parameters and use the structure at
> > the
> > fbc_activate function. Of course, it's still possible to access
> > everything from dev_priv in those functions, but IMHO the new code
> > will be harder to break.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 22 ++++++-
> > drivers/gpu/drm/i915/intel_fbc.c | 131 ++++++++++++++++++++++-----
> > ------------
> > 2 files changed, 93 insertions(+), 60 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 33217a4..aa9c35e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -909,11 +909,9 @@ struct i915_fbc {
> > * it's the outer lock when overlapping with stolen_lock.
> > */
> > struct mutex lock;
> > unsigned threshold;
> > - unsigned int fb_id;
> > unsigned int possible_framebuffer_bits;
> > unsigned int busy_bits;
> > struct intel_crtc *crtc;
> > - int y;
> >
> > struct drm_mm_node compressed_fb;
> > struct drm_mm_node *compressed_llb;
> > @@ -923,6 +921,24 @@ struct i915_fbc {
> > bool enabled;
> > bool active;
> >
> > + struct intel_fbc_reg_params {
> > + struct {
> > + enum pipe pipe;
> > + enum plane plane;
> > + unsigned int fence_y_offset;
> > + } crtc;
> > +
> > + struct {
> > + u64 ggtt_offset;
> > + uint32_t id;
> On a casual glance it looks like this fb.id is write-only now, but
> the whole struct is being compared.
It is read during intel_fbc_reg_params_equal(), so we're still
comparing it. I tried to be conservative and just remove the fb.id
comparison later, just to make bisecting easier in case something
happens. But we remove fb.id in patch 22/25.
>
> It might be worth making a note about that.
> ~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
