On Fri, Mar 04, 2022 at 05:10:33PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 03, 2022 at 02:41:23PM -0800, Navare, Manasi wrote:
> > On Wed, Feb 23, 2022 at 03:13:15PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >
> > > When using bigjoiner it's useful to know the offset of each
> > > individual pipe in the whole set of joined pipes. Let's include
> > > that information in our PIPESRC rectangle. With this we can make
> > > the plane clipping code blissfully unaware of bigjoiner usage, as
> > > all we have to do is remove the pipe's offset from the final plane
> > > destination coordinates.
> > >
> > > v2: Use intel_bigjoiner_num_pipes()
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > ---
> > > .../gpu/drm/i915/display/intel_atomic_plane.c | 7 +++---
> > > drivers/gpu/drm/i915/display/intel_cursor.c | 8 ++++---
> > > drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++++
> > > drivers/gpu/drm/i915/display/intel_overlay.c | 22 +++++++++----------
> > > 4 files changed, 40 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > index 3cbf66146da0..92ae4eebc62f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > @@ -824,10 +824,6 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state,
> > > return -ERANGE;
> > > }
> > >
> > > - /* right side of the image is on the slave crtc, adjust dst to match */
> > > - if (intel_crtc_is_bigjoiner_slave(crtc_state))
> > > - drm_rect_translate(dst, -drm_rect_width(&crtc_state->pipe_src), 0);
> > > -
> > > /*
> > > * FIXME: This might need further adjustment for seamless scaling
> > > * with phase information, for the 2p2 and 2p1 scenarios.
> > > @@ -844,6 +840,9 @@ int intel_atomic_plane_check_clipping(struct intel_plane_state *plane_state,
> > > return -EINVAL;
> > > }
> > >
> > > + /* final plane coordinates will be relative to the plane's pipe */
> > > + drm_rect_translate(dst, -clip->x1, -clip->y1);
> > > +
> > > return 0;
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> > > index da6cf0515164..9279e2783e7e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> > > @@ -152,9 +152,11 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
> > > /* Use the unclipped src/dst rectangles, which we program to hw */
> > > plane_state->uapi.src = src;
> > > plane_state->uapi.dst = dst;
> > > - if (intel_crtc_is_bigjoiner_slave(crtc_state))
> > > - drm_rect_translate(&plane_state->uapi.dst,
> > > - -drm_rect_width(&crtc_state->pipe_src), 0);
> > > +
> > > + /* final plane coordinates will be relative to the plane's pipe */
> > > + drm_rect_translate(&plane_state->uapi.dst,
> > > + -crtc_state->pipe_src.x1,
> > > + -crtc_state->pipe_src.y1);
> > >
> > > ret = intel_cursor_check_surface(plane_state);
> > > if (ret)
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 7a09bb33c1eb..a9c15f27b948 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -3204,6 +3204,23 @@ static void intel_get_transcoder_timings(struct intel_crtc *crtc,
> > > }
> > > }
> > >
> > > +static void intel_bigjoiner_adjust_pipe_src(struct intel_crtc_state *crtc_state)
> > > +{
> > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > + int num_pipes = intel_bigjoiner_num_pipes(crtc_state);
> > > + enum pipe master_pipe, pipe = crtc->pipe;
> > > + int width;
> > > +
> > > + if (num_pipes < 2)
> > > + return;
> > > +
> > > + master_pipe = bigjoiner_master_pipe(crtc_state);
> > > + width = drm_rect_width(&crtc_state->pipe_src);
> > > +
> > > + drm_rect_translate_to(&crtc_state->pipe_src,
> > > + (pipe - master_pipe) * width, 0);
> > > +}
> > > +
> > > static void intel_get_pipe_src_size(struct intel_crtc *crtc,
> > > struct intel_crtc_state *pipe_config)
> > > {
> > > @@ -3216,6 +3233,8 @@ static void intel_get_pipe_src_size(struct intel_crtc *crtc,
> > > drm_rect_init(&pipe_config->pipe_src, 0, 0,
> > > REG_FIELD_GET(PIPESRC_WIDTH_MASK, tmp) + 1,
> > > REG_FIELD_GET(PIPESRC_HEIGHT_MASK, tmp) + 1);
> > > +
> > > + intel_bigjoiner_adjust_pipe_src(pipe_config);
> > > }
> > >
> > > static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
> > > @@ -5853,6 +5872,8 @@ intel_modeset_pipe_config_late(struct intel_crtc_state *crtc_state)
> > > struct drm_connector *connector;
> > > int i;
> > >
> > > + intel_bigjoiner_adjust_pipe_src(crtc_state);
> > > +
> > > for_each_new_connector_in_state(&state->base, connector,
> > > conn_state, i) {
> > > struct intel_encoder *encoder =
> > > diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
> > > index 631e1f1dc5e6..ee46561b5ae8 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_overlay.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
> > > @@ -958,21 +958,21 @@ static void update_pfit_vscale_ratio(struct intel_overlay *overlay)
> > > static int check_overlay_dst(struct intel_overlay *overlay,
> > > struct drm_intel_overlay_put_image *rec)
> > > {
> > > - const struct intel_crtc_state *pipe_config =
> > > + const struct intel_crtc_state *crtc_state =
> > > overlay->crtc->config;
> > > - int pipe_src_w = drm_rect_width(&pipe_config->pipe_src);
> > > - int pipe_src_h = drm_rect_height(&pipe_config->pipe_src);
> > > + struct drm_rect req, clipped;
> > >
> > > - if (rec->dst_height == 0 || rec->dst_width == 0)
> > > - return -EINVAL;
> > > + drm_rect_init(&req, rec->dst_x, rec->dst_y,
> > > + rec->dst_width, rec->dst_height);
> > > +
> > > + clipped = req;
> > > + drm_rect_intersect(&clipped, &crtc_state->pipe_src);
> > >
> > > - if (rec->dst_x < pipe_src_w &&
> > > - rec->dst_x + rec->dst_width <= pipe_src_w &&
> > > - rec->dst_y < pipe_src_h &&
> > > - rec->dst_y + rec->dst_height <= pipe_src_h)
> > > - return 0;
> > > - else
> > > + if (!drm_rect_visible(&clipped) ||
> > > + !drm_rect_equals(&clipped, &req))
> > > return -EINVAL;
> > > +
> > > + return 0;
> >
> > I dont quite understand what we are trying to do here with the clipped ?
> > Can you elaborate a bit what this function does?
>
> It checks whether the userspace provided coordinates were fully
> within the pipe source rectangle. If not we return an error.
>
> I suppose technically we should be checking against the uapi
> hdisplay/vdisplay since this is uapi stuff, but no bigjoiner
> on these old platforms so this will effectively do the same
> thing.
>
Okay sounds good then
Reviewed-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
Manasi
> --
> Ville Syrjälä
> Intel
