| Message ID | 1455203142-10894-1-git-send-email-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 11 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote: > In case of BXT DSI we are updating the CPU_TRANSCODER > with appropriate value. > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_display.c | 5 +++++ > 2 files changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 65a2cd0..ef4b376 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -122,6 +122,8 @@ enum transcoder { > TRANSCODER_B, > TRANSCODER_C, > TRANSCODER_EDP, > + TRANSCODER_MIPI_A, > + TRANSCODER_MIPI_C, > I915_MAX_TRANSCODERS > }; > #define transcoder_name(t) ((t) + 'A') > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index e47a75c..9715056 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7851,6 +7851,11 @@ static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc, > enc_to_intel_dsi(&encoder->base); > pipe_config->pipe_bpp = > dsi_pixel_format_bpp(intel_dsi->pixel_format); > + > + if (intel_dsi->ports & (1 << PORT_A)) > + pipe_config->cpu_transcoder = TRANSCODER_MIPI_A; > + else > + pipe_config->cpu_transcoder = TRANSCODER_MIPI_C; > } > } As I've said in my review of the previous version of the patch [1], you can't just add transcoder identifiers and expect everything to magically work. The current assumption is there are at most transcoders A, B, C, and eDP. We use them to index registers. Not all registers have a corresponding DSI transcoder variant, and if they do, they are not uniformly spread in the register offset space. See haswell_crtc_disable for an example where things go wrong. There are others. I do not know what the best approach here would be; it is obvious to me this can't work. BR, Jani. [1] http://mid.gmane.org/87powceh4r.fsf@intel.com
On Friday 19 February 2016 02:37 PM, Jani Nikula wrote: > On Thu, 11 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote: >> In case of BXT DSI we are updating the CPU_TRANSCODER >> with appropriate value. >> >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 2 ++ >> drivers/gpu/drm/i915/intel_display.c | 5 +++++ >> 2 files changed, 7 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 65a2cd0..ef4b376 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -122,6 +122,8 @@ enum transcoder { >> TRANSCODER_B, >> TRANSCODER_C, >> TRANSCODER_EDP, >> + TRANSCODER_MIPI_A, >> + TRANSCODER_MIPI_C, >> I915_MAX_TRANSCODERS >> }; >> #define transcoder_name(t) ((t) + 'A') >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index e47a75c..9715056 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -7851,6 +7851,11 @@ static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc, >> enc_to_intel_dsi(&encoder->base); >> pipe_config->pipe_bpp = >> dsi_pixel_format_bpp(intel_dsi->pixel_format); >> + >> + if (intel_dsi->ports & (1 << PORT_A)) >> + pipe_config->cpu_transcoder = TRANSCODER_MIPI_A; >> + else >> + pipe_config->cpu_transcoder = TRANSCODER_MIPI_C; >> } >> } > As I've said in my review of the previous version of the patch [1], you > can't just add transcoder identifiers and expect everything to magically > work. > > The current assumption is there are at most transcoders A, B, C, and > eDP. We use them to index registers. Not all registers have a > corresponding DSI transcoder variant, and if they do, they are not > uniformly spread in the register offset space. See haswell_crtc_disable > for an example where things go wrong. There are others. > > I do not know what the best approach here would be; it is obvious to me > this can't work. As correcting the CPU transcoder value alone wont solve our problem. In the next patch set I am adding corresponding POWER_DOMAIN bits and adding them to the respective POWER wells. This will enable generic power well management calls to map the cpu transcoders to the corresponding powerwell and help them to maintain them as per the need. Hope that will address the concern raised here. > > BR, > Jani. > > > [1] http://mid.gmane.org/87powceh4r.fsf@intel.com > > >
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 65a2cd0..ef4b376 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -122,6 +122,8 @@ enum transcoder { TRANSCODER_B, TRANSCODER_C, TRANSCODER_EDP, + TRANSCODER_MIPI_A, + TRANSCODER_MIPI_C, I915_MAX_TRANSCODERS }; #define transcoder_name(t) ((t) + 'A') diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e47a75c..9715056 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7851,6 +7851,11 @@ static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc, enc_to_intel_dsi(&encoder->base); pipe_config->pipe_bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format); + + if (intel_dsi->ports & (1 << PORT_A)) + pipe_config->cpu_transcoder = TRANSCODER_MIPI_A; + else + pipe_config->cpu_transcoder = TRANSCODER_MIPI_C; } }