Re: [PATCH 07/17] drm/i915: Add and implement the debugfs 'show' functions for Displayport compliance

Daniel Vetter
2014-12-17T20:11:37Z

On Tue, Dec 16, 2014 at 05:00:38PM -0200, Paulo Zanoni wrote:
> 2014-12-10 21:53 GMT-02:00 Todd Previte <tprevite@gmail.com>:
> > + if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) {
> > + if (intel_encoder->new_crtc) {
> > + crtc_config = &intel_encoder->new_crtc->config;
> > + pipe_bpp = crtc_config->pipe_bpp;
> > + mode = &crtc_config->adjusted_mode;
> > + hres = mode->hdisplay;
> > + vres = mode->vdisplay;
> > + } else if (intel_encoder->base.crtc) {
> > + icrtc = to_intel_crtc(intel_encoder->base.crtc);
> > + pipe_bpp = icrtc->config.pipe_bpp;
> > + mode = &icrtc->config.adjusted_mode;
> > + hres = mode->hdisplay;
> > + vres = mode->vdisplay;
> > + } else {
> > + pipe_bpp = 0;
> > + hres = vres = 0;
> > + }
>
> Why do you have this new_crtc vs current_crtc vs no crtc distinction
> here? Is it really necessary? Shouldn't the "DP testing debugfs
> protocol" establish when exactly the information should be queried,
> and then give some errors in case information is being requested at
> the wrong time?

Presuming adequate locking exists (haven't checked tbh) new_* pointers always
match the equivalent base pointers. new_ pointers/structures are
exclusively for modeset code (and specifically state computation). I guess
the new_crtc case needs to be dropped here.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch