| Message ID | 1463059132-1720-3-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 12, 2016 at 04:18:50PM +0300, Imre Deak wrote: > Code checkers may complain about the explicit casts between different > enum types, so add comments for known-valid cases to help future > triaging of such complaints. > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index b60d9b6..763726d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9294,6 +9294,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, > ironlake_get_fdi_m_n_config(crtc, pipe_config); > > if (HAS_PCH_IBX(dev_priv)) { > + /* the pipe->pll mapping is fixed */ Should perhaps say 'pch transocder->pll mapping' here. > pll_id = (enum intel_dpll_id) crtc->pipe; > } else { > tmp = I915_READ(PCH_DPLL_SEL); > @@ -9844,6 +9845,10 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc, > enum intel_display_power_domain power_domain; > u32 tmp; > > + /* > + * The transcoder->pipe mapping is fixed with the exception of the eDP Saying 'pipe->transcoder mapping' would feel more natural to me since that's the way the data flows. Also that's the way assignment in the code happens. But I can live with the current thing too, so Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + * transcoder handled below. > + */ > pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe; > > /* > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 2016-05-12 at 16:30 +0300, Ville Syrjälä wrote: > On Thu, May 12, 2016 at 04:18:50PM +0300, Imre Deak wrote: > > Code checkers may complain about the explicit casts between different > > enum types, so add comments for known-valid cases to help future > > triaging of such complaints. > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index b60d9b6..763726d 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -9294,6 +9294,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, > > ironlake_get_fdi_m_n_config(crtc, pipe_config); > > > > if (HAS_PCH_IBX(dev_priv)) { > > + /* the pipe->pll mapping is fixed */ > > Should perhaps say 'pch transocder->pll mapping' here. Ok, but then 'pipe->pch transcoder and pch transcoder->pll mapping is fixed'? > > pll_id = (enum intel_dpll_id) crtc->pipe; > > } else { > > tmp = I915_READ(PCH_DPLL_SEL); > > @@ -9844,6 +9845,10 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc, > > enum intel_display_power_domain power_domain; > > u32 tmp; > > > > + /* > > + * The transcoder->pipe mapping is fixed with the exception of the eDP > > Saying 'pipe->transcoder mapping' would feel more natural to me > since that's the way the data flows. Also that's the way assignment in > the code happens. Ok, will change that. > But I can live with the current thing too, so > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > + * transcoder handled below. > > + */ > > pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe; > > > > /* > > -- > > 2.5.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Thu, May 12, 2016 at 05:00:56PM +0300, Imre Deak wrote: > On Thu, 2016-05-12 at 16:30 +0300, Ville Syrjälä wrote: > > On Thu, May 12, 2016 at 04:18:50PM +0300, Imre Deak wrote: > > > Code checkers may complain about the explicit casts between different > > > enum types, so add comments for known-valid cases to help future > > > triaging of such complaints. > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index b60d9b6..763726d 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -9294,6 +9294,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, > > > ironlake_get_fdi_m_n_config(crtc, pipe_config); > > > > > > if (HAS_PCH_IBX(dev_priv)) { > > > + /* the pipe->pll mapping is fixed */ > > > > Should perhaps say 'pch transocder->pll mapping' here. > > Ok, but then 'pipe->pch transcoder and pch transcoder->pll mapping is > fixed'? I guess you could say that. We don't have a separate enum for pch transcoder since it's the same as the pipe always, and thus we usually use 'pipe' to denote the pch transcoder as well. Well, except on HSW/BDW which only have pch transcoder A, and sometimes on those platforms we use enum transcoder also for the pch transcoder. So it's a huge mess really, and I was considering adding a pch transcoder enum at some point but so far I didn't get bored enough to actually do it. > > > > pll_id = (enum intel_dpll_id) crtc->pipe; > > > } else { > > > tmp = I915_READ(PCH_DPLL_SEL); > > > @@ -9844,6 +9845,10 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc, > > > enum intel_display_power_domain power_domain; > > > u32 tmp; > > > > > > + /* > > > + * The transcoder->pipe mapping is fixed with the exception of the eDP > > > > Saying 'pipe->transcoder mapping' would feel more natural to me > > since that's the way the data flows. Also that's the way assignment in > > the code happens. > > Ok, will change that. > > > But I can live with the current thing too, so > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > + * transcoder handled below. > > > + */ > > > pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe; > > > > > > /* > > > -- > > > 2.5.0 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b60d9b6..763726d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9294,6 +9294,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, ironlake_get_fdi_m_n_config(crtc, pipe_config); if (HAS_PCH_IBX(dev_priv)) { + /* the pipe->pll mapping is fixed */ pll_id = (enum intel_dpll_id) crtc->pipe; } else { tmp = I915_READ(PCH_DPLL_SEL); @@ -9844,6 +9845,10 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc, enum intel_display_power_domain power_domain; u32 tmp; + /* + * The transcoder->pipe mapping is fixed with the exception of the eDP + * transcoder handled below. + */ pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe; /*
Code checkers may complain about the explicit casts between different enum types, so add comments for known-valid cases to help future triaging of such complaints. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 5 +++++ 1 file changed, 5 insertions(+)