| Message ID | 20210330184254.6290-2-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915: Extract intel_adjusted_rate() | expand |
On Tue, 30 Mar 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Replace the hand rolled pfit downscale calculations with > intel_adjusted_rate(). > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > .../gpu/drm/i915/display/intel_atomic_plane.c | 6 ++--- > .../gpu/drm/i915/display/intel_atomic_plane.h | 4 ++++ > drivers/gpu/drm/i915/display/intel_display.c | 23 +++++-------------- > 3 files changed, 13 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > index 3f830b70b0c1..5f0a5ea474eb 100644 > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > @@ -133,9 +133,9 @@ intel_plane_destroy_state(struct drm_plane *plane, > kfree(plane_state); > } > > -static unsigned int intel_adjusted_rate(const struct drm_rect *src, > - const struct drm_rect *dst, > - unsigned int rate) > +unsigned int intel_adjusted_rate(const struct drm_rect *src, > + const struct drm_rect *dst, > + unsigned int rate) > { > unsigned int src_w, src_h, dst_w, dst_h; > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h > index 5c78a087ed86..dc4d05e75e1c 100644 > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h > @@ -10,6 +10,7 @@ > > struct drm_plane; > struct drm_property; > +struct drm_rect; > struct intel_atomic_state; > struct intel_crtc; > struct intel_crtc_state; > @@ -18,6 +19,9 @@ struct intel_plane_state; > > extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; > > +unsigned int intel_adjusted_rate(const struct drm_rect *src, > + const struct drm_rect *dst, > + unsigned int rate); > unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state); > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index d74b263c5f4e..472e691286c6 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3978,7 +3978,7 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) > static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state) > { > u32 pixel_rate = crtc_state->hw.pipe_mode.crtc_clock; > - unsigned int pipe_w, pipe_h, pfit_w, pfit_h; > + struct drm_rect src; > > /* > * We only use IF-ID interlacing. If we ever use > @@ -3988,23 +3988,12 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state) > if (!crtc_state->pch_pfit.enabled) > return pixel_rate; > > - pipe_w = crtc_state->pipe_src_w; > - pipe_h = crtc_state->pipe_src_h; > + drm_rect_init(&src, 0, 0, > + crtc_state->pipe_src_w << 16, > + crtc_state->pipe_src_h << 16); > > - pfit_w = drm_rect_width(&crtc_state->pch_pfit.dst); > - pfit_h = drm_rect_height(&crtc_state->pch_pfit.dst); > - > - if (pipe_w < pfit_w) > - pipe_w = pfit_w; So this is src_w = max(src_w, dst_w) and gets turned into dst_w = min(src_w, dst_w) instead? Ditto for _h. Does it end up being the same thing after the division? BR, Jani. > - if (pipe_h < pfit_h) > - pipe_h = pfit_h; > - > - if (drm_WARN_ON(crtc_state->uapi.crtc->dev, > - !pfit_w || !pfit_h)) > - return pixel_rate; > - > - return div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h), > - pfit_w * pfit_h); > + return intel_adjusted_rate(&src, &crtc_state->pch_pfit.dst, > + pixel_rate); > } > > static void intel_mode_from_crtc_timings(struct drm_display_mode *mode,
On Thu, Apr 01, 2021 at 03:55:20PM +0300, Jani Nikula wrote: > On Tue, 30 Mar 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Replace the hand rolled pfit downscale calculations with > > intel_adjusted_rate(). > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > .../gpu/drm/i915/display/intel_atomic_plane.c | 6 ++--- > > .../gpu/drm/i915/display/intel_atomic_plane.h | 4 ++++ > > drivers/gpu/drm/i915/display/intel_display.c | 23 +++++-------------- > > 3 files changed, 13 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > > index 3f830b70b0c1..5f0a5ea474eb 100644 > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > > @@ -133,9 +133,9 @@ intel_plane_destroy_state(struct drm_plane *plane, > > kfree(plane_state); > > } > > > > -static unsigned int intel_adjusted_rate(const struct drm_rect *src, > > - const struct drm_rect *dst, > > - unsigned int rate) > > +unsigned int intel_adjusted_rate(const struct drm_rect *src, > > + const struct drm_rect *dst, > > + unsigned int rate) > > { > > unsigned int src_w, src_h, dst_w, dst_h; > > > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h > > index 5c78a087ed86..dc4d05e75e1c 100644 > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h > > @@ -10,6 +10,7 @@ > > > > struct drm_plane; > > struct drm_property; > > +struct drm_rect; > > struct intel_atomic_state; > > struct intel_crtc; > > struct intel_crtc_state; > > @@ -18,6 +19,9 @@ struct intel_plane_state; > > > > extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; > > > > +unsigned int intel_adjusted_rate(const struct drm_rect *src, > > + const struct drm_rect *dst, > > + unsigned int rate); > > unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state, > > const struct intel_plane_state *plane_state); > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index d74b263c5f4e..472e691286c6 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -3978,7 +3978,7 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) > > static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state) > > { > > u32 pixel_rate = crtc_state->hw.pipe_mode.crtc_clock; > > - unsigned int pipe_w, pipe_h, pfit_w, pfit_h; > > + struct drm_rect src; > > > > /* > > * We only use IF-ID interlacing. If we ever use > > @@ -3988,23 +3988,12 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state) > > if (!crtc_state->pch_pfit.enabled) > > return pixel_rate; > > > > - pipe_w = crtc_state->pipe_src_w; > > - pipe_h = crtc_state->pipe_src_h; > > + drm_rect_init(&src, 0, 0, > > + crtc_state->pipe_src_w << 16, > > + crtc_state->pipe_src_h << 16); > > > > - pfit_w = drm_rect_width(&crtc_state->pch_pfit.dst); > > - pfit_h = drm_rect_height(&crtc_state->pch_pfit.dst); > > - > > - if (pipe_w < pfit_w) > > - pipe_w = pfit_w; > > So this is src_w = max(src_w, dst_w) and gets turned into dst_w = > min(src_w, dst_w) instead? Ditto for _h. Does it end up being the same > thing after the division? Yes. The min/max just gets rid of the upscaling case, ie. causes the division to be just x/x==1 when dst>src. Doesn't matter if we use the min or max approach to achieve that result.
On Thu, 01 Apr 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, Apr 01, 2021 at 03:55:20PM +0300, Jani Nikula wrote: >> On Tue, 30 Mar 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> > >> > Replace the hand rolled pfit downscale calculations with >> > intel_adjusted_rate(). >> > >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> > --- >> > .../gpu/drm/i915/display/intel_atomic_plane.c | 6 ++--- >> > .../gpu/drm/i915/display/intel_atomic_plane.h | 4 ++++ >> > drivers/gpu/drm/i915/display/intel_display.c | 23 +++++-------------- >> > 3 files changed, 13 insertions(+), 20 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c >> > index 3f830b70b0c1..5f0a5ea474eb 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c >> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c >> > @@ -133,9 +133,9 @@ intel_plane_destroy_state(struct drm_plane *plane, >> > kfree(plane_state); >> > } >> > >> > -static unsigned int intel_adjusted_rate(const struct drm_rect *src, >> > - const struct drm_rect *dst, >> > - unsigned int rate) >> > +unsigned int intel_adjusted_rate(const struct drm_rect *src, >> > + const struct drm_rect *dst, >> > + unsigned int rate) >> > { >> > unsigned int src_w, src_h, dst_w, dst_h; >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h >> > index 5c78a087ed86..dc4d05e75e1c 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h >> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h >> > @@ -10,6 +10,7 @@ >> > >> > struct drm_plane; >> > struct drm_property; >> > +struct drm_rect; >> > struct intel_atomic_state; >> > struct intel_crtc; >> > struct intel_crtc_state; >> > @@ -18,6 +19,9 @@ struct intel_plane_state; >> > >> > extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; >> > >> > +unsigned int intel_adjusted_rate(const struct drm_rect *src, >> > + const struct drm_rect *dst, >> > + unsigned int rate); >> > unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state, >> > const struct intel_plane_state *plane_state); >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> > index d74b263c5f4e..472e691286c6 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_display.c >> > +++ b/drivers/gpu/drm/i915/display/intel_display.c >> > @@ -3978,7 +3978,7 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) >> > static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state) >> > { >> > u32 pixel_rate = crtc_state->hw.pipe_mode.crtc_clock; >> > - unsigned int pipe_w, pipe_h, pfit_w, pfit_h; >> > + struct drm_rect src; >> > >> > /* >> > * We only use IF-ID interlacing. If we ever use >> > @@ -3988,23 +3988,12 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state) >> > if (!crtc_state->pch_pfit.enabled) >> > return pixel_rate; >> > >> > - pipe_w = crtc_state->pipe_src_w; >> > - pipe_h = crtc_state->pipe_src_h; >> > + drm_rect_init(&src, 0, 0, >> > + crtc_state->pipe_src_w << 16, >> > + crtc_state->pipe_src_h << 16); >> > >> > - pfit_w = drm_rect_width(&crtc_state->pch_pfit.dst); >> > - pfit_h = drm_rect_height(&crtc_state->pch_pfit.dst); >> > - >> > - if (pipe_w < pfit_w) >> > - pipe_w = pfit_w; >> >> So this is src_w = max(src_w, dst_w) and gets turned into dst_w = >> min(src_w, dst_w) instead? Ditto for _h. Does it end up being the same >> thing after the division? > > Yes. The min/max just gets rid of the upscaling case, > ie. causes the division to be just x/x==1 when dst>src. > Doesn't matter if we use the min or max approach to > achieve that result. Ok, thanks. Reviewed-by: Jani Nikula <jani.nikula@intel.com>
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c index 3f830b70b0c1..5f0a5ea474eb 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c @@ -133,9 +133,9 @@ intel_plane_destroy_state(struct drm_plane *plane, kfree(plane_state); } -static unsigned int intel_adjusted_rate(const struct drm_rect *src, - const struct drm_rect *dst, - unsigned int rate) +unsigned int intel_adjusted_rate(const struct drm_rect *src, + const struct drm_rect *dst, + unsigned int rate) { unsigned int src_w, src_h, dst_w, dst_h; diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h index 5c78a087ed86..dc4d05e75e1c 100644 --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h @@ -10,6 +10,7 @@ struct drm_plane; struct drm_property; +struct drm_rect; struct intel_atomic_state; struct intel_crtc; struct intel_crtc_state; @@ -18,6 +19,9 @@ struct intel_plane_state; extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; +unsigned int intel_adjusted_rate(const struct drm_rect *src, + const struct drm_rect *dst, + unsigned int rate); unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state); diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index d74b263c5f4e..472e691286c6 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3978,7 +3978,7 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state) { u32 pixel_rate = crtc_state->hw.pipe_mode.crtc_clock; - unsigned int pipe_w, pipe_h, pfit_w, pfit_h; + struct drm_rect src; /* * We only use IF-ID interlacing. If we ever use @@ -3988,23 +3988,12 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state) if (!crtc_state->pch_pfit.enabled) return pixel_rate; - pipe_w = crtc_state->pipe_src_w; - pipe_h = crtc_state->pipe_src_h; + drm_rect_init(&src, 0, 0, + crtc_state->pipe_src_w << 16, + crtc_state->pipe_src_h << 16); - pfit_w = drm_rect_width(&crtc_state->pch_pfit.dst); - pfit_h = drm_rect_height(&crtc_state->pch_pfit.dst); - - if (pipe_w < pfit_w) - pipe_w = pfit_w; - if (pipe_h < pfit_h) - pipe_h = pfit_h; - - if (drm_WARN_ON(crtc_state->uapi.crtc->dev, - !pfit_w || !pfit_h)) - return pixel_rate; - - return div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h), - pfit_w * pfit_h); + return intel_adjusted_rate(&src, &crtc_state->pch_pfit.dst, + pixel_rate); } static void intel_mode_from_crtc_timings(struct drm_display_mode *mode,