Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state

2024-02-14 Thread Dmitry Baryshkov
On Wed, 14 Feb 2024 at 21:39, Ville Syrjälä
 wrote:
>
> On Wed, Feb 14, 2024 at 09:17:06PM +0200, Dmitry Baryshkov wrote:
> > On Wed, 14 Feb 2024 at 20:47, Ville Syrjälä
> >  wrote:
> > >
> > > On Wed, Feb 14, 2024 at 08:37:02PM +0200, Ville Syrjälä wrote:
> > > > On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> > > > > The helper drm_atomic_helper_check_plane_state() runs several checks 
> > > > > on
> > > > > plane src and dst rectangles, including the check whether required
> > > > > scaling fits into the required margins. The msm driver would benefit
> > > > > from having a function that does all these checks except the scaling
> > > > > one. Split them into a new helper called
> > > > > drm_atomic_helper_check_plane_noscale().
> > > >
> > > > What's the point in eliminating a nop scaling check?
> > >
> > > Actually, what are you even doing in there? Are you saying that
> > > the hardware has absolutely no limits on how much it can scale
> > > in either direction?
> >
> > No, I'm just saying that the scaling ability depends on the rotation
> > and other plane properties. So I had to separate the basic plane
> > checks and the scaling check.
> > Basic (noscale) plane check source and destination rectangles, etc.
> > After that the driver identifies possible hardware pipe usage and
> > after that it can perform a scaling check.
>
> Hmm. We have sport of similar situation in i915 where we pick a scaler
> much later and so don't know the exact scaling limits at the time when
> we do this check. But we opted to pass the lower/upper bounds of the
> scaling limits instead. That will guarantee that at least completely
> illegal values are rejected as early as possible, and so we don't have
> to worry about running into them later on.

Ack, I'll follow this approach then.

-- 
With best wishes
Dmitry


Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state

2024-02-14 Thread Ville Syrjälä
On Wed, Feb 14, 2024 at 09:17:06PM +0200, Dmitry Baryshkov wrote:
> On Wed, 14 Feb 2024 at 20:47, Ville Syrjälä
>  wrote:
> >
> > On Wed, Feb 14, 2024 at 08:37:02PM +0200, Ville Syrjälä wrote:
> > > On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> > > > The helper drm_atomic_helper_check_plane_state() runs several checks on
> > > > plane src and dst rectangles, including the check whether required
> > > > scaling fits into the required margins. The msm driver would benefit
> > > > from having a function that does all these checks except the scaling
> > > > one. Split them into a new helper called
> > > > drm_atomic_helper_check_plane_noscale().
> > >
> > > What's the point in eliminating a nop scaling check?
> >
> > Actually, what are you even doing in there? Are you saying that
> > the hardware has absolutely no limits on how much it can scale
> > in either direction?
> 
> No, I'm just saying that the scaling ability depends on the rotation
> and other plane properties. So I had to separate the basic plane
> checks and the scaling check.
> Basic (noscale) plane check source and destination rectangles, etc.
> After that the driver identifies possible hardware pipe usage and
> after that it can perform a scaling check.

Hmm. We have sport of similar situation in i915 where we pick a scaler
much later and so don't know the exact scaling limits at the time when
we do this check. But we opted to pass the lower/upper bounds of the
scaling limits instead. That will guarantee that at least completely
illegal values are rejected as early as possible, and so we don't have
to worry about running into them later on.

-- 
Ville Syrjälä
Intel


Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state

2024-02-14 Thread Dmitry Baryshkov
On Wed, 14 Feb 2024 at 20:47, Ville Syrjälä
 wrote:
>
> On Wed, Feb 14, 2024 at 08:37:02PM +0200, Ville Syrjälä wrote:
> > On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> > > The helper drm_atomic_helper_check_plane_state() runs several checks on
> > > plane src and dst rectangles, including the check whether required
> > > scaling fits into the required margins. The msm driver would benefit
> > > from having a function that does all these checks except the scaling
> > > one. Split them into a new helper called
> > > drm_atomic_helper_check_plane_noscale().
> >
> > What's the point in eliminating a nop scaling check?
>
> Actually, what are you even doing in there? Are you saying that
> the hardware has absolutely no limits on how much it can scale
> in either direction?

No, I'm just saying that the scaling ability depends on the rotation
and other plane properties. So I had to separate the basic plane
checks and the scaling check.
Basic (noscale) plane check source and destination rectangles, etc.
After that the driver identifies possible hardware pipe usage and
after that it can perform a scaling check.

>
> >
> > >
> > > Signed-off-by: Dmitry Baryshkov 
> > > ---
> > >  drivers/gpu/drm/drm_atomic_helper.c | 110 ++--
> > >  include/drm/drm_atomic_helper.h |   7 ++
> > >  2 files changed, 96 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 292e38eb6218..2d7dd66181c9 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct 
> > > drm_encoder *encoder,
> > >  EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
> > >
> > >  /**
> > > - * drm_atomic_helper_check_plane_state() - Check plane state for validity
> > > + * drm_atomic_helper_check_plane_noscale() - Check plane state for 
> > > validity
> > >   * @plane_state: plane state to check
> > >   * @crtc_state: CRTC state to check
> > > - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > > - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> > >   * @can_position: is it legal to position the plane such that it
> > >   *doesn't cover the entire CRTC?  This will generally
> > >   *only be false for primary planes.
> > > @@ -845,19 +843,16 @@ 
> > > EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
> > >   * RETURNS:
> > >   * Zero if update appears valid, error code on failure
> > >   */
> > > -int drm_atomic_helper_check_plane_state(struct drm_plane_state 
> > > *plane_state,
> > > -   const struct drm_crtc_state 
> > > *crtc_state,
> > > -   int min_scale,
> > > -   int max_scale,
> > > -   bool can_position,
> > > -   bool can_update_disabled)
> > > +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state 
> > > *plane_state,
> > > + const struct drm_crtc_state 
> > > *crtc_state,
> > > + bool can_position,
> > > + bool can_update_disabled)
> > >  {
> > > struct drm_framebuffer *fb = plane_state->fb;
> > > struct drm_rect *src = _state->src;
> > > struct drm_rect *dst = _state->dst;
> > > unsigned int rotation = plane_state->rotation;
> > > struct drm_rect clip = {};
> > > -   int hscale, vscale;
> > >
> > > WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> > >
> > > @@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct 
> > > drm_plane_state *plane_state,
> > >
> > > drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> > >
> > > -   /* Check scaling */
> > > -   hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> > > -   vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> > > -   if (hscale < 0 || vscale < 0) {
> > > -   drm_dbg_kms(plane_state->plane->dev,
> > > -   "Invalid scaling of plane\n");
> > > -   drm_rect_debug_print("src: ", _state->src, true);
> > > -   drm_rect_debug_print("dst: ", _state->dst, false);
> > > -   return -ERANGE;
> > > -   }
> > > -
> > > if (crtc_state->enable)
> > > drm_mode_get_hv_timing(_state->mode, , );
> > >
> > > @@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct 
> > > drm_plane_state *plane_state,
> > >
> > > return 0;
> > >  }
> > > +EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
> > > +
> > > +/**
> > > + * drm_atomic_helper_check_plane_scale() - Check whether plane can be 
> > > scaled
> > > + * @plane_state: plane state to check
> > > + * @min_scale: minimum @src:@dest scaling factor in 16.16 

Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state

2024-02-14 Thread Ville Syrjälä
On Wed, Feb 14, 2024 at 08:37:02PM +0200, Ville Syrjälä wrote:
> On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> > The helper drm_atomic_helper_check_plane_state() runs several checks on
> > plane src and dst rectangles, including the check whether required
> > scaling fits into the required margins. The msm driver would benefit
> > from having a function that does all these checks except the scaling
> > one. Split them into a new helper called
> > drm_atomic_helper_check_plane_noscale().
> 
> What's the point in eliminating a nop scaling check?

Actually, what are you even doing in there? Are you saying that
the hardware has absolutely no limits on how much it can scale
in either direction?

> 
> > 
> > Signed-off-by: Dmitry Baryshkov 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 110 ++--
> >  include/drm/drm_atomic_helper.h |   7 ++
> >  2 files changed, 96 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 292e38eb6218..2d7dd66181c9 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct 
> > drm_encoder *encoder,
> >  EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
> >  
> >  /**
> > - * drm_atomic_helper_check_plane_state() - Check plane state for validity
> > + * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
> >   * @plane_state: plane state to check
> >   * @crtc_state: CRTC state to check
> > - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> >   * @can_position: is it legal to position the plane such that it
> >   *doesn't cover the entire CRTC?  This will generally
> >   *only be false for primary planes.
> > @@ -845,19 +843,16 @@ 
> > EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
> >   * RETURNS:
> >   * Zero if update appears valid, error code on failure
> >   */
> > -int drm_atomic_helper_check_plane_state(struct drm_plane_state 
> > *plane_state,
> > -   const struct drm_crtc_state *crtc_state,
> > -   int min_scale,
> > -   int max_scale,
> > -   bool can_position,
> > -   bool can_update_disabled)
> > +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state 
> > *plane_state,
> > + const struct drm_crtc_state 
> > *crtc_state,
> > + bool can_position,
> > + bool can_update_disabled)
> >  {
> > struct drm_framebuffer *fb = plane_state->fb;
> > struct drm_rect *src = _state->src;
> > struct drm_rect *dst = _state->dst;
> > unsigned int rotation = plane_state->rotation;
> > struct drm_rect clip = {};
> > -   int hscale, vscale;
> >  
> > WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> >  
> > @@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct 
> > drm_plane_state *plane_state,
> >  
> > drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> >  
> > -   /* Check scaling */
> > -   hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> > -   vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> > -   if (hscale < 0 || vscale < 0) {
> > -   drm_dbg_kms(plane_state->plane->dev,
> > -   "Invalid scaling of plane\n");
> > -   drm_rect_debug_print("src: ", _state->src, true);
> > -   drm_rect_debug_print("dst: ", _state->dst, false);
> > -   return -ERANGE;
> > -   }
> > -
> > if (crtc_state->enable)
> > drm_mode_get_hv_timing(_state->mode, , );
> >  
> > @@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct 
> > drm_plane_state *plane_state,
> >  
> > return 0;
> >  }
> > +EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
> > +
> > +/**
> > + * drm_atomic_helper_check_plane_scale() - Check whether plane can be 
> > scaled
> > + * @plane_state: plane state to check
> > + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> > + *
> > + * Checks that a desired plane scale fits into the min_scale..max_scale
> > + * boundaries.
> > + * Drivers that provide their own plane handling rather than 
> > helper-provided
> > + * implementations may still wish to call this function to avoid 
> > duplication of
> > + * error checking code.
> > + *
> > + * RETURNS:
> > + * Zero if update appears valid, error code on failure
> > + */
> > +int drm_atomic_helper_check_plane_scale(struct drm_plane_state 
> > *plane_state,
> > +   

Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state

2024-02-14 Thread Ville Syrjälä
On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> The helper drm_atomic_helper_check_plane_state() runs several checks on
> plane src and dst rectangles, including the check whether required
> scaling fits into the required margins. The msm driver would benefit
> from having a function that does all these checks except the scaling
> one. Split them into a new helper called
> drm_atomic_helper_check_plane_noscale().

What's the point in eliminating a nop scaling check?

> 
> Signed-off-by: Dmitry Baryshkov 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 110 ++--
>  include/drm/drm_atomic_helper.h |   7 ++
>  2 files changed, 96 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 292e38eb6218..2d7dd66181c9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct 
> drm_encoder *encoder,
>  EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
>  
>  /**
> - * drm_atomic_helper_check_plane_state() - Check plane state for validity
> + * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
>   * @plane_state: plane state to check
>   * @crtc_state: CRTC state to check
> - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
>   * @can_position: is it legal to position the plane such that it
>   *doesn't cover the entire CRTC?  This will generally
>   *only be false for primary planes.
> @@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
>   * RETURNS:
>   * Zero if update appears valid, error code on failure
>   */
> -int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> - const struct drm_crtc_state *crtc_state,
> - int min_scale,
> - int max_scale,
> - bool can_position,
> - bool can_update_disabled)
> +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state 
> *plane_state,
> +   const struct drm_crtc_state 
> *crtc_state,
> +   bool can_position,
> +   bool can_update_disabled)
>  {
>   struct drm_framebuffer *fb = plane_state->fb;
>   struct drm_rect *src = _state->src;
>   struct drm_rect *dst = _state->dst;
>   unsigned int rotation = plane_state->rotation;
>   struct drm_rect clip = {};
> - int hscale, vscale;
>  
>   WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
>  
> @@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct 
> drm_plane_state *plane_state,
>  
>   drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
>  
> - /* Check scaling */
> - hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> - vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> - if (hscale < 0 || vscale < 0) {
> - drm_dbg_kms(plane_state->plane->dev,
> - "Invalid scaling of plane\n");
> - drm_rect_debug_print("src: ", _state->src, true);
> - drm_rect_debug_print("dst: ", _state->dst, false);
> - return -ERANGE;
> - }
> -
>   if (crtc_state->enable)
>   drm_mode_get_hv_timing(_state->mode, , );
>  
> @@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct 
> drm_plane_state *plane_state,
>  
>   return 0;
>  }
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
> +
> +/**
> + * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
> + * @plane_state: plane state to check
> + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> + *
> + * Checks that a desired plane scale fits into the min_scale..max_scale
> + * boundaries.
> + * Drivers that provide their own plane handling rather than helper-provided
> + * implementations may still wish to call this function to avoid duplication 
> of
> + * error checking code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
> + int min_scale,
> + int max_scale)
> +{
> + struct drm_framebuffer *fb = plane_state->fb;
> + struct drm_rect src;
> + struct drm_rect dst;
> + int hscale, vscale;
> +
> + if (!plane_state->visible)
> + return 0;
> +
> + src = drm_plane_state_src(plane_state);
> + dst = 

Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state

2024-02-14 Thread Abhinav Kumar




On 9/13/2023 10:06 PM, Dmitry Baryshkov wrote:

The helper drm_atomic_helper_check_plane_state() runs several checks on
plane src and dst rectangles, including the check whether required
scaling fits into the required margins. The msm driver would benefit
from having a function that does all these checks except the scaling
one. Split them into a new helper called
drm_atomic_helper_check_plane_noscale().

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/drm_atomic_helper.c | 110 ++--
  include/drm/drm_atomic_helper.h |   7 ++
  2 files changed, 96 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 292e38eb6218..2d7dd66181c9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct 
drm_encoder *encoder,
  EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
  
  /**

- * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
   * @plane_state: plane state to check
   * @crtc_state: CRTC state to check
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
   * @can_position: is it legal to position the plane such that it
   *doesn't cover the entire CRTC?  This will generally
   *only be false for primary planes.
@@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
   * RETURNS:
   * Zero if update appears valid, error code on failure
   */
-int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
-   const struct drm_crtc_state *crtc_state,
-   int min_scale,
-   int max_scale,
-   bool can_position,
-   bool can_update_disabled)
+int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state 
*crtc_state,
+ bool can_position,
+ bool can_update_disabled)
  {
struct drm_framebuffer *fb = plane_state->fb;
struct drm_rect *src = _state->src;
struct drm_rect *dst = _state->dst;
unsigned int rotation = plane_state->rotation;
struct drm_rect clip = {};
-   int hscale, vscale;
  
  	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
  
@@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
  
  	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
  


Do we need to do the rotation even for noscale before validation?


-   /* Check scaling */
-   hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
-   vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
-   if (hscale < 0 || vscale < 0) {
-   drm_dbg_kms(plane_state->plane->dev,
-   "Invalid scaling of plane\n");
-   drm_rect_debug_print("src: ", _state->src, true);
-   drm_rect_debug_print("dst: ", _state->dst, false);
-   return -ERANGE;
-   }
-
if (crtc_state->enable)
drm_mode_get_hv_timing(_state->mode, , );
  
@@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
  
  	return 0;

  }
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
+
+/**
+ * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
+ * @plane_state: plane state to check
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ *
+ * Checks that a desired plane scale fits into the min_scale..max_scale
+ * boundaries.
+ * Drivers that provide their own plane handling rather than helper-provided
+ * implementations may still wish to call this function to avoid duplication of
+ * error checking code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
+   int min_scale,
+   int max_scale)
+{
+   struct drm_framebuffer *fb = plane_state->fb;
+   struct drm_rect src;
+   struct drm_rect dst;
+   int hscale, vscale;
+
+   if (!plane_state->visible)
+   return 0;
+
+   src = drm_plane_state_src(plane_state);
+   dst = drm_plane_state_dest(plane_state);
+
+   drm_rect_rotate(, fb->width << 16, fb->height << 16, 
plane_state->rotation);
+


Does this need to be accompanied by 

[Freedreno] [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state

2023-09-13 Thread Dmitry Baryshkov
The helper drm_atomic_helper_check_plane_state() runs several checks on
plane src and dst rectangles, including the check whether required
scaling fits into the required margins. The msm driver would benefit
from having a function that does all these checks except the scaling
one. Split them into a new helper called
drm_atomic_helper_check_plane_noscale().

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/drm_atomic_helper.c | 110 ++--
 include/drm/drm_atomic_helper.h |   7 ++
 2 files changed, 96 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 292e38eb6218..2d7dd66181c9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct 
drm_encoder *encoder,
 EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
 
 /**
- * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
  * @plane_state: plane state to check
  * @crtc_state: CRTC state to check
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
  * @can_position: is it legal to position the plane such that it
  *doesn't cover the entire CRTC?  This will generally
  *only be false for primary planes.
@@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
  * RETURNS:
  * Zero if update appears valid, error code on failure
  */
-int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
-   const struct drm_crtc_state *crtc_state,
-   int min_scale,
-   int max_scale,
-   bool can_position,
-   bool can_update_disabled)
+int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state 
*crtc_state,
+ bool can_position,
+ bool can_update_disabled)
 {
struct drm_framebuffer *fb = plane_state->fb;
struct drm_rect *src = _state->src;
struct drm_rect *dst = _state->dst;
unsigned int rotation = plane_state->rotation;
struct drm_rect clip = {};
-   int hscale, vscale;
 
WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
 
@@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct 
drm_plane_state *plane_state,
 
drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
 
-   /* Check scaling */
-   hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
-   vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
-   if (hscale < 0 || vscale < 0) {
-   drm_dbg_kms(plane_state->plane->dev,
-   "Invalid scaling of plane\n");
-   drm_rect_debug_print("src: ", _state->src, true);
-   drm_rect_debug_print("dst: ", _state->dst, false);
-   return -ERANGE;
-   }
-
if (crtc_state->enable)
drm_mode_get_hv_timing(_state->mode, , );
 
@@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct 
drm_plane_state *plane_state,
 
return 0;
 }
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
+
+/**
+ * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
+ * @plane_state: plane state to check
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ *
+ * Checks that a desired plane scale fits into the min_scale..max_scale
+ * boundaries.
+ * Drivers that provide their own plane handling rather than helper-provided
+ * implementations may still wish to call this function to avoid duplication of
+ * error checking code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
+   int min_scale,
+   int max_scale)
+{
+   struct drm_framebuffer *fb = plane_state->fb;
+   struct drm_rect src;
+   struct drm_rect dst;
+   int hscale, vscale;
+
+   if (!plane_state->visible)
+   return 0;
+
+   src = drm_plane_state_src(plane_state);
+   dst = drm_plane_state_dest(plane_state);
+
+   drm_rect_rotate(, fb->width << 16, fb->height << 16, 
plane_state->rotation);
+
+   hscale = drm_rect_calc_hscale(, , min_scale, max_scale);
+   vscale = drm_rect_calc_vscale(, , min_scale, max_scale);
+   if (hscale < 0 || vscale < 0)