| Message ID | 20170714193822.12121-1-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 14, 2017 at 04:38:22PM -0300, Paulo Zanoni wrote: > I wrote this code an year and a half ago and I couldn't exactly > remember the main differences of these two structures when reviewing a > new FBC patch. Add some comments to help explain what's the purpose of > each struct. > > For the record, the original commits are: > b183b3f14395 ("drm/i915/fbc: introduce struct intel_fbc_reg_params") > aaf78d276ba0 ("drm/i915/fbc: introduce struct intel_fbc_state_cache") > > Cc: Praveen Paneri <praveen.paneri@intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Make them kerneldoc? It allows in-line comment, and maybe we'll have proper kerneldoc for all our structs eventually. Either way Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/i915_drv.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a1eeb489..271402c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1063,6 +1063,11 @@ struct intel_fbc { > bool underrun_detected; > struct work_struct underrun_work; > > + /* > + * Due to the atomic rules we can't access some structures without the > + * appropriate locking, so we cache information here in order to avoid > + * these problems. > + */ > struct intel_fbc_state_cache { > struct i915_vma *vma; > > @@ -1084,6 +1089,13 @@ struct intel_fbc { > } fb; > } state_cache; > > + /* > + * This structure contains everything that's relevant to program the > + * hardware registers. When we want to figure out if we need to disable > + * and re-enable FBC for a new configuration we just check if there's > + * something different in the struct. The genx_fbc_activate functions > + * are supposed to read from it in order to program the registers. > + */ > struct intel_fbc_reg_params { > struct i915_vma *vma; > > -- > 2.9.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a1eeb489..271402c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1063,6 +1063,11 @@ struct intel_fbc { bool underrun_detected; struct work_struct underrun_work; + /* + * Due to the atomic rules we can't access some structures without the + * appropriate locking, so we cache information here in order to avoid + * these problems. + */ struct intel_fbc_state_cache { struct i915_vma *vma; @@ -1084,6 +1089,13 @@ struct intel_fbc { } fb; } state_cache; + /* + * This structure contains everything that's relevant to program the + * hardware registers. When we want to figure out if we need to disable + * and re-enable FBC for a new configuration we just check if there's + * something different in the struct. The genx_fbc_activate functions + * are supposed to read from it in order to program the registers. + */ struct intel_fbc_reg_params { struct i915_vma *vma;
I wrote this code an year and a half ago and I couldn't exactly remember the main differences of these two structures when reviewing a new FBC patch. Add some comments to help explain what's the purpose of each struct. For the record, the original commits are: b183b3f14395 ("drm/i915/fbc: introduce struct intel_fbc_reg_params") aaf78d276ba0 ("drm/i915/fbc: introduce struct intel_fbc_state_cache") Cc: Praveen Paneri <praveen.paneri@intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)