| Message ID | 20230321011821.635977-3-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,v2,01/13] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state | expand |
On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: > Take into account the plane rotation and flipping when calculating src > positions for the wide plane parts. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Do we need to have a fixes tag for this? This means we dont consider rotation while calculating src position today which is a bug? > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 ++++++++++++++--------- > 1 file changed, 17 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > index 2e63eb0a2f3f..d43e04fc4578 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > @@ -887,16 +887,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > return -EINVAL; > } > > - pipe_cfg->src_rect = new_plane_state->src; > - > - /* state->src is 16.16, src_rect is not */ > - pipe_cfg->src_rect.x1 >>= 16; > - pipe_cfg->src_rect.x2 >>= 16; > - pipe_cfg->src_rect.y1 >>= 16; > - pipe_cfg->src_rect.y2 >>= 16; > - > - pipe_cfg->dst_rect = new_plane_state->dst; > - > fb_rect.x2 = new_plane_state->fb->width; > fb_rect.y2 = new_plane_state->fb->height; > > @@ -912,6 +902,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > max_linewidth = pdpu->catalog->caps->max_linewidth; > > + /* state->src is 16.16, src_rect is not */ > + drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); > + > + pipe_cfg->dst_rect = new_plane_state->dst; > + > + drm_rect_rotate(&pipe_cfg->src_rect, > + new_plane_state->fb->width, new_plane_state->fb->height, > + new_plane_state->rotation); > + > if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) { > /* > * In parallel multirect case only the half of the usual width > @@ -959,6 +958,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; > } > > + drm_rect_rotate_inv(&pipe_cfg->src_rect, > + new_plane_state->fb->width, new_plane_state->fb->height, > + new_plane_state->rotation); > + if (r_pipe->sspp) Dont you need to check for if (r_pipe_cfg) here and not if (r_pipe->sspp) because parameter you are passing is the r_pipe_cfg to drm_rect_rotate_inv(). So we rotated the pipe_cfg once, then rotated_inv it to restore the rectangle to its original state, but r_pipe_cfg's rectangle was never rotated as it was not allocated before this function so it will remain in inverse rotated state now right? > + drm_rect_rotate_inv(&r_pipe_cfg->src_rect, > + new_plane_state->fb->width, new_plane_state->fb->height, > + new_plane_state->rotation); > + > ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt); > if (ret) > return ret;
On Sat, 13 May 2023 at 01:12, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: > > Take into account the plane rotation and flipping when calculating src > > positions for the wide plane parts. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Do we need to have a fixes tag for this? This means we dont consider > rotation while calculating src position today which is a bug? Hmm, I thought that I had a check forbidding rotation with the current approach, but I don't see it. Most probably I thought about it and then forgot to add it. The proper fix should be to disallow it for static SSPP case. I'll include the patch into v3. > > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 ++++++++++++++--------- > > 1 file changed, 17 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > index 2e63eb0a2f3f..d43e04fc4578 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > @@ -887,16 +887,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > return -EINVAL; > > } > > > > - pipe_cfg->src_rect = new_plane_state->src; > > - > > - /* state->src is 16.16, src_rect is not */ > > - pipe_cfg->src_rect.x1 >>= 16; > > - pipe_cfg->src_rect.x2 >>= 16; > > - pipe_cfg->src_rect.y1 >>= 16; > > - pipe_cfg->src_rect.y2 >>= 16; > > - > > - pipe_cfg->dst_rect = new_plane_state->dst; > > - > > fb_rect.x2 = new_plane_state->fb->width; > > fb_rect.y2 = new_plane_state->fb->height; > > > > @@ -912,6 +902,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > > > max_linewidth = pdpu->catalog->caps->max_linewidth; > > > > + /* state->src is 16.16, src_rect is not */ > > + drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); > > + > > + pipe_cfg->dst_rect = new_plane_state->dst; > > + > > + drm_rect_rotate(&pipe_cfg->src_rect, > > + new_plane_state->fb->width, new_plane_state->fb->height, > > + new_plane_state->rotation); > > + > > if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) { > > /* > > * In parallel multirect case only the half of the usual width > > @@ -959,6 +958,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; > > } > > > > + drm_rect_rotate_inv(&pipe_cfg->src_rect, > > + new_plane_state->fb->width, new_plane_state->fb->height, > > + new_plane_state->rotation); > > + if (r_pipe->sspp) > > Dont you need to check for if (r_pipe_cfg) here and not if > (r_pipe->sspp) because parameter you are passing is the r_pipe_cfg to > drm_rect_rotate_inv(). Of course not. r_pipe_cfg is a pointer to the field in pstate. We know that it can not be NULL. > > So we rotated the pipe_cfg once, then rotated_inv it to restore the > rectangle to its original state, but r_pipe_cfg's rectangle was never > rotated as it was not allocated before this function so it will remain > in inverse rotated state now right? No. r_pipe_cfg is set beforehand to the half of the rotated pipe_cfg. > > + drm_rect_rotate_inv(&r_pipe_cfg->src_rect, > > + new_plane_state->fb->width, new_plane_state->fb->height, > > + new_plane_state->rotation); > > + > > ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt); > > if (ret) > > return ret; -- With best wishes Dmitry
On 5/14/2023 10:01 AM, Dmitry Baryshkov wrote: > On Sat, 13 May 2023 at 01:12, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: >>> Take into account the plane rotation and flipping when calculating src >>> positions for the wide plane parts. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> >> Do we need to have a fixes tag for this? This means we dont consider >> rotation while calculating src position today which is a bug? > > Hmm, I thought that I had a check forbidding rotation with the current > approach, but I don't see it. Most probably I thought about it and > then forgot to add it. > The proper fix should be to disallow it for static SSPP case. I'll > include the patch into v3. > >> >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 ++++++++++++++--------- >>> 1 file changed, 17 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >>> index 2e63eb0a2f3f..d43e04fc4578 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >>> @@ -887,16 +887,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, >>> return -EINVAL; >>> } >>> >>> - pipe_cfg->src_rect = new_plane_state->src; >>> - >>> - /* state->src is 16.16, src_rect is not */ >>> - pipe_cfg->src_rect.x1 >>= 16; >>> - pipe_cfg->src_rect.x2 >>= 16; >>> - pipe_cfg->src_rect.y1 >>= 16; >>> - pipe_cfg->src_rect.y2 >>= 16; >>> - >>> - pipe_cfg->dst_rect = new_plane_state->dst; >>> - >>> fb_rect.x2 = new_plane_state->fb->width; >>> fb_rect.y2 = new_plane_state->fb->height; >>> >>> @@ -912,6 +902,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, >>> >>> max_linewidth = pdpu->catalog->caps->max_linewidth; >>> >>> + /* state->src is 16.16, src_rect is not */ >>> + drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); >>> + >>> + pipe_cfg->dst_rect = new_plane_state->dst; >>> + >>> + drm_rect_rotate(&pipe_cfg->src_rect, >>> + new_plane_state->fb->width, new_plane_state->fb->height, >>> + new_plane_state->rotation); >>> + >>> if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) { >>> /* >>> * In parallel multirect case only the half of the usual width >>> @@ -959,6 +958,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, >>> r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; >>> } >>> >>> + drm_rect_rotate_inv(&pipe_cfg->src_rect, >>> + new_plane_state->fb->width, new_plane_state->fb->height, >>> + new_plane_state->rotation); >>> + if (r_pipe->sspp) >> >> Dont you need to check for if (r_pipe_cfg) here and not if >> (r_pipe->sspp) because parameter you are passing is the r_pipe_cfg to >> drm_rect_rotate_inv(). > > Of course not. r_pipe_cfg is a pointer to the field in pstate. We know > that it can not be NULL. > Ack, and my bad for not checking that r_pipe_cfg points to a field in pstate but .... it was just weird though that you are checking for r_pipe->sspp before calling a method which really doesnt care if its null or not. How about you use drm_rect_visible(r_pipe_cfg->src_rect) If its not set, it wont be visible too. >> >> So we rotated the pipe_cfg once, then rotated_inv it to restore the >> rectangle to its original state, but r_pipe_cfg's rectangle was never >> rotated as it was not allocated before this function so it will remain >> in inverse rotated state now right? > > No. r_pipe_cfg is set beforehand to the half of the rotated pipe_cfg. > Ok i got it now. Instead of directly operating on the plane_state's rectangle which makes you to invert again why not just use a temporary drm_rect which stores the rotated pipe_cfg->src_rect. That way you dont have to invert anything? >>> + drm_rect_rotate_inv(&r_pipe_cfg->src_rect, >>> + new_plane_state->fb->width, new_plane_state->fb->height, >>> + new_plane_state->rotation); >>> + >>> ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt); >>> if (ret) >>> return ret; > > > > -- > With best wishes > Dmitry
On Mon, 15 May 2023 at 21:45, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 5/14/2023 10:01 AM, Dmitry Baryshkov wrote: > > On Sat, 13 May 2023 at 01:12, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >> > >> > >> > >> On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: > >>> Take into account the plane rotation and flipping when calculating src > >>> positions for the wide plane parts. > >>> > >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >> > >> Do we need to have a fixes tag for this? This means we dont consider > >> rotation while calculating src position today which is a bug? > > > > Hmm, I thought that I had a check forbidding rotation with the current > > approach, but I don't see it. Most probably I thought about it and > > then forgot to add it. > > The proper fix should be to disallow it for static SSPP case. I'll > > include the patch into v3. > > > >> > >>> --- > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 ++++++++++++++--------- > >>> 1 file changed, 17 insertions(+), 10 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > >>> index 2e63eb0a2f3f..d43e04fc4578 100644 > >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > >>> @@ -887,16 +887,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > >>> return -EINVAL; > >>> } > >>> > >>> - pipe_cfg->src_rect = new_plane_state->src; > >>> - > >>> - /* state->src is 16.16, src_rect is not */ > >>> - pipe_cfg->src_rect.x1 >>= 16; > >>> - pipe_cfg->src_rect.x2 >>= 16; > >>> - pipe_cfg->src_rect.y1 >>= 16; > >>> - pipe_cfg->src_rect.y2 >>= 16; > >>> - > >>> - pipe_cfg->dst_rect = new_plane_state->dst; > >>> - > >>> fb_rect.x2 = new_plane_state->fb->width; > >>> fb_rect.y2 = new_plane_state->fb->height; > >>> > >>> @@ -912,6 +902,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > >>> > >>> max_linewidth = pdpu->catalog->caps->max_linewidth; > >>> > >>> + /* state->src is 16.16, src_rect is not */ > >>> + drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); > >>> + > >>> + pipe_cfg->dst_rect = new_plane_state->dst; > >>> + > >>> + drm_rect_rotate(&pipe_cfg->src_rect, > >>> + new_plane_state->fb->width, new_plane_state->fb->height, > >>> + new_plane_state->rotation); > >>> + > >>> if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) { > >>> /* > >>> * In parallel multirect case only the half of the usual width > >>> @@ -959,6 +958,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > >>> r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; > >>> } > >>> > >>> + drm_rect_rotate_inv(&pipe_cfg->src_rect, > >>> + new_plane_state->fb->width, new_plane_state->fb->height, > >>> + new_plane_state->rotation); > >>> + if (r_pipe->sspp) > >> > >> Dont you need to check for if (r_pipe_cfg) here and not if > >> (r_pipe->sspp) because parameter you are passing is the r_pipe_cfg to > >> drm_rect_rotate_inv(). > > > > Of course not. r_pipe_cfg is a pointer to the field in pstate. We know > > that it can not be NULL. > > > > Ack, and my bad for not checking that r_pipe_cfg points to a field in > pstate but .... it was just weird though that you are checking for > r_pipe->sspp before calling a method which really doesnt care if its > null or not. How about you use drm_rect_visible(r_pipe_cfg->src_rect) > > If its not set, it wont be visible too. I think it is better for the uniformity to check for r_pipe->sspp: this is the condition that is used all over the driver to check that r_pipe is used. > > >> > >> So we rotated the pipe_cfg once, then rotated_inv it to restore the > >> rectangle to its original state, but r_pipe_cfg's rectangle was never > >> rotated as it was not allocated before this function so it will remain > >> in inverse rotated state now right? > > > > No. r_pipe_cfg is set beforehand to the half of the rotated pipe_cfg. > > > > Ok i got it now. Instead of directly operating on the plane_state's > rectangle which makes you to invert again why not just use a temporary > drm_rect which stores the rotated pipe_cfg->src_rect. That way you dont > have to invert anything? I don't think this will work. I explicitly rotate & invert rotation to get correct coordinates for both source and destination rectangles. Doing it otherwise would require us to manually implement this in the DPU driver. > > >>> + drm_rect_rotate_inv(&r_pipe_cfg->src_rect, > >>> + new_plane_state->fb->width, new_plane_state->fb->height, > >>> + new_plane_state->rotation); > >>> + > >>> ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt); > >>> if (ret) > >>> return ret; > > > > > > > > -- > > With best wishes > > Dmitry
On 5/15/2023 12:12 PM, Dmitry Baryshkov wrote: > On Mon, 15 May 2023 at 21:45, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 5/14/2023 10:01 AM, Dmitry Baryshkov wrote: >>> On Sat, 13 May 2023 at 01:12, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >>>> >>>> >>>> >>>> On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: >>>>> Take into account the plane rotation and flipping when calculating src >>>>> positions for the wide plane parts. >>>>> >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>> >>>> Do we need to have a fixes tag for this? This means we dont consider >>>> rotation while calculating src position today which is a bug? >>> >>> Hmm, I thought that I had a check forbidding rotation with the current >>> approach, but I don't see it. Most probably I thought about it and >>> then forgot to add it. >>> The proper fix should be to disallow it for static SSPP case. I'll >>> include the patch into v3. >>> >>>> >>>>> --- >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 ++++++++++++++--------- >>>>> 1 file changed, 17 insertions(+), 10 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >>>>> index 2e63eb0a2f3f..d43e04fc4578 100644 >>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >>>>> @@ -887,16 +887,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, >>>>> return -EINVAL; >>>>> } >>>>> >>>>> - pipe_cfg->src_rect = new_plane_state->src; >>>>> - >>>>> - /* state->src is 16.16, src_rect is not */ >>>>> - pipe_cfg->src_rect.x1 >>= 16; >>>>> - pipe_cfg->src_rect.x2 >>= 16; >>>>> - pipe_cfg->src_rect.y1 >>= 16; >>>>> - pipe_cfg->src_rect.y2 >>= 16; >>>>> - >>>>> - pipe_cfg->dst_rect = new_plane_state->dst; >>>>> - >>>>> fb_rect.x2 = new_plane_state->fb->width; >>>>> fb_rect.y2 = new_plane_state->fb->height; >>>>> >>>>> @@ -912,6 +902,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, >>>>> >>>>> max_linewidth = pdpu->catalog->caps->max_linewidth; >>>>> >>>>> + /* state->src is 16.16, src_rect is not */ >>>>> + drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); >>>>> + >>>>> + pipe_cfg->dst_rect = new_plane_state->dst; >>>>> + >>>>> + drm_rect_rotate(&pipe_cfg->src_rect, >>>>> + new_plane_state->fb->width, new_plane_state->fb->height, >>>>> + new_plane_state->rotation); >>>>> + >>>>> if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) { >>>>> /* >>>>> * In parallel multirect case only the half of the usual width >>>>> @@ -959,6 +958,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, >>>>> r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; >>>>> } >>>>> >>>>> + drm_rect_rotate_inv(&pipe_cfg->src_rect, >>>>> + new_plane_state->fb->width, new_plane_state->fb->height, >>>>> + new_plane_state->rotation); >>>>> + if (r_pipe->sspp) >>>> >>>> Dont you need to check for if (r_pipe_cfg) here and not if >>>> (r_pipe->sspp) because parameter you are passing is the r_pipe_cfg to >>>> drm_rect_rotate_inv(). >>> >>> Of course not. r_pipe_cfg is a pointer to the field in pstate. We know >>> that it can not be NULL. >>> >> >> Ack, and my bad for not checking that r_pipe_cfg points to a field in >> pstate but .... it was just weird though that you are checking for >> r_pipe->sspp before calling a method which really doesnt care if its >> null or not. How about you use drm_rect_visible(r_pipe_cfg->src_rect) >> >> If its not set, it wont be visible too. > > I think it is better for the uniformity to check for r_pipe->sspp: > this is the condition that is used all over the driver to check that > r_pipe is used. > hmmm .... okay .... not entirely convinced this was the right way to begin with then because some places do need a valid sspp for the function getting called so thats fine but some do not. its incorrect uniformity, but I am not going to complain about it now. will think of cleaning it up once this lands. >> >>>> >>>> So we rotated the pipe_cfg once, then rotated_inv it to restore the >>>> rectangle to its original state, but r_pipe_cfg's rectangle was never >>>> rotated as it was not allocated before this function so it will remain >>>> in inverse rotated state now right? >>> >>> No. r_pipe_cfg is set beforehand to the half of the rotated pipe_cfg. >>> >> >> Ok i got it now. Instead of directly operating on the plane_state's >> rectangle which makes you to invert again why not just use a temporary >> drm_rect which stores the rotated pipe_cfg->src_rect. That way you dont >> have to invert anything? > > I don't think this will work. I explicitly rotate & invert rotation to > get correct coordinates for both source and destination rectangles. > Doing it otherwise would require us to manually implement this in the > DPU driver. > Ok got it, i guess this will need more changes within the if (src_width > max_width) .... this is fine then. Will ack this once i finish reviews on the others. >> >>>>> + drm_rect_rotate_inv(&r_pipe_cfg->src_rect, >>>>> + new_plane_state->fb->width, new_plane_state->fb->height, >>>>> + new_plane_state->rotation); >>>>> + >>>>> ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt); >>>>> if (ret) >>>>> return ret; >>> >>> >>> >>> -- >>> With best wishes >>> Dmitry > > >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 2e63eb0a2f3f..d43e04fc4578 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -887,16 +887,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, return -EINVAL; } - pipe_cfg->src_rect = new_plane_state->src; - - /* state->src is 16.16, src_rect is not */ - pipe_cfg->src_rect.x1 >>= 16; - pipe_cfg->src_rect.x2 >>= 16; - pipe_cfg->src_rect.y1 >>= 16; - pipe_cfg->src_rect.y2 >>= 16; - - pipe_cfg->dst_rect = new_plane_state->dst; - fb_rect.x2 = new_plane_state->fb->width; fb_rect.y2 = new_plane_state->fb->height; @@ -912,6 +902,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, max_linewidth = pdpu->catalog->caps->max_linewidth; + /* state->src is 16.16, src_rect is not */ + drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); + + pipe_cfg->dst_rect = new_plane_state->dst; + + drm_rect_rotate(&pipe_cfg->src_rect, + new_plane_state->fb->width, new_plane_state->fb->height, + new_plane_state->rotation); + if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) { /* * In parallel multirect case only the half of the usual width @@ -959,6 +958,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; } + drm_rect_rotate_inv(&pipe_cfg->src_rect, + new_plane_state->fb->width, new_plane_state->fb->height, + new_plane_state->rotation); + if (r_pipe->sspp) + drm_rect_rotate_inv(&r_pipe_cfg->src_rect, + new_plane_state->fb->width, new_plane_state->fb->height, + new_plane_state->rotation); + ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt); if (ret) return ret;
Take into account the plane rotation and flipping when calculating src positions for the wide plane parts. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 ++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-)