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

Todd Previte
2015-02-18T16:45:33Z

On 12/17/14 1:12 PM, Daniel Vetter wrote:
> 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
I tested this again with the new_crtc case removed and it appears to
work correctly. So the new_crtc case has been removed. The changes have
been integrated into the monster patch.

-T