| Message ID | 1382382620-23405-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 21, 2013 at 09:10:20PM +0200, Daniel Vetter wrote: > Adding stuff to the bottom of struct drm_i915_driver_private is > nowadays considered uncool. > > Cc: Damien Lespiau <damien.lespiau@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Ville was suggesting moving the pipe_crc struct into intel_crtc. Might be even better! But anyway: Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
On Mon, Oct 21, 2013 at 10:47 PM, Damien Lespiau <damien.lespiau@intel.com> wrote: > On Mon, Oct 21, 2013 at 09:10:20PM +0200, Daniel Vetter wrote: >> Adding stuff to the bottom of struct drm_i915_driver_private is >> nowadays considered uncool. >> >> Cc: Damien Lespiau <damien.lespiau@intel.com> >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Ville was suggesting moving the pipe_crc struct into intel_crtc. Might > be even better! But anyway: We enable interrupts before we allocate the intel_crtc structs. So if we'd store the CRC stuff in there we'd need to be a bit more paranoid with clearing hw state (since every time we trust the BIOS we end up burning our hands). So I think this is safer ... -Daniel
On Mon, Oct 21, 2013 at 10:54:16PM +0200, Daniel Vetter wrote: > On Mon, Oct 21, 2013 at 10:47 PM, Damien Lespiau > <damien.lespiau@intel.com> wrote: > > On Mon, Oct 21, 2013 at 09:10:20PM +0200, Daniel Vetter wrote: > >> Adding stuff to the bottom of struct drm_i915_driver_private is > >> nowadays considered uncool. > >> > >> Cc: Damien Lespiau <damien.lespiau@intel.com> > >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Ville was suggesting moving the pipe_crc struct into intel_crtc. Might > > be even better! But anyway: > > We enable interrupts before we allocate the intel_crtc structs. So if > we'd store the CRC stuff in there we'd need to be a bit more paranoid > with clearing hw state (since every time we trust the BIOS we end up > burning our hands). So I think this is safer ... FLIP_DONE etc. interrupts already depend on the crtc being there. Not sure why this is different.
On Tue, Oct 22, 2013 at 10:15 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > FLIP_DONE etc. interrupts already depend on the crtc being there. Not > sure why this is different. I'd say we've simply lucked out thus far. E.g. on vlv the init_clock_gating flushes the primary plane and so casues a flip done interrupt. But right now we call that clock gating stuff after crtc init so we're good. So my concern isn't that this can't be done but that it's fragile. And right now we don't have any infrastructure to help with the driver boostrapping at all, but we've had tons of cases where things blew up in funny way. So good ideas welcome. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index faface9..2e1e884 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1380,6 +1380,10 @@ typedef struct drm_i915_private { struct drm_crtc *pipe_to_crtc_mapping[3]; wait_queue_head_t pending_flip_queue; +#ifdef CONFIG_DEBUG_FS + struct intel_pipe_crc pipe_crc[I915_MAX_PIPES]; +#endif + int num_shared_dpll; struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; struct intel_ddi_plls ddi_plls; @@ -1460,10 +1464,6 @@ typedef struct drm_i915_private { struct i915_dri1_state dri1; /* Old ums support infrastructure, same warning applies. */ struct i915_ums_state ums; - -#ifdef CONFIG_DEBUG_FS - struct intel_pipe_crc pipe_crc[I915_MAX_PIPES]; -#endif } drm_i915_private_t; static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
Adding stuff to the bottom of struct drm_i915_driver_private is nowadays considered uncool. Cc: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_drv.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)