Re: [PATCH 01/35] drm/i915: Add scaled paramater to update_sprite_watermarks()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2013/7/30 Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>:
> On Tue, Jul 30, 2013 at 03:26:12PM -0300, Paulo Zanoni wrote:
>> 2013/7/5  <ville.syrjala@xxxxxxxxxxxxxxx>:
>> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> >
>> > Fro calculating watermarks we want to know whether sprites are
>>
>> "Fro"
>>
>> > scaled. Pass that information to update_sprite_watermarks() so that
>> > eventually we may do some watermark pre-computing.
>>
>> On this patch you're also renaming some variables from "enable" to
>> "enabled", but not all of them. You should probably either rename them
>> all, or none. Example: intel_update_sprite_watermarks definition at
>> intel_drv.h says "bool enabled", but the implementation inside
>> intel_pm.c says "bool enable", but there are also other examples.
>
> The idea was to use "enabled" consistently, but it seems I messed up.
> I can fix that up.
>
>>
>> Besides the styling detail the patch looks correct, so if Daniel/Ville
>> consider my suggestion is just a bikeshed, Reviewed-by: Paulo Zanoni
>> <paulo.r.zanoni@xxxxxxxxx>.
>>
>> And another bikeshed would be to create variables called "scaled"
>> inside the update_plane funcs :)
>
> You mean "bool scaled = crtc_w != src_w || crtc_h != src_h;" or so?

Yes, but I just realized that will trigger a rebase of many other
patches of your series... So you may skip it if you want. Maybe do it
in the end, or not at all.

>
> Yeah I suppose could make the code a bit easier to parse.
>
>>
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.h     |  2 +-
>> >  drivers/gpu/drm/i915/intel_drv.h    |  7 ++++---
>> >  drivers/gpu/drm/i915/intel_pm.c     | 13 +++++++------
>> >  drivers/gpu/drm/i915/intel_sprite.c | 11 +++++++----
>> >  4 files changed, 19 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > index fd0f589..99eb980 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -361,7 +361,7 @@ struct drm_i915_display_funcs {
>> >         void (*update_wm)(struct drm_device *dev);
>> >         void (*update_sprite_wm)(struct drm_device *dev, int pipe,
>> >                                  uint32_t sprite_width, int pixel_size,
>> > -                                bool enable);
>> > +                                bool enable, bool scaled);
>> >         void (*modeset_global_resources)(struct drm_device *dev);
>> >         /* Returns the active state of the crtc, and if the crtc is active,
>> >          * fills out the pipe-config with the hw state. */
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > index 5dfc1a0..3371ecc 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -353,7 +353,8 @@ struct intel_plane {
>> >          * for the watermark calculations. Currently only Haswell uses this.
>> >          */
>> >         struct {
>> > -               bool enable;
>> > +               bool enabled;
>> > +               bool scaled;
>> >                 uint8_t bytes_per_pixel;
>> >                 uint32_t horiz_pixels;
>> >         } wm;
>> > @@ -772,8 +773,8 @@ extern void intel_ddi_init(struct drm_device *dev, enum port port);
>> >  /* For use by IVB LP watermark workaround in intel_sprite.c */
>> >  extern void intel_update_watermarks(struct drm_device *dev);
>> >  extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
>> > -                                          uint32_t sprite_width,
>> > -                                          int pixel_size, bool enable);
>> > +                                          uint32_t sprite_width, int pixel_size,
>> > +                                          bool enabled, bool scaled);
>> >
>> >  extern unsigned long intel_gen4_compute_page_offset(int *x, int *y,
>> >                                                     unsigned int tiling_mode,
>> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> > index 7cfd3b7..beca186 100644
>> > --- a/drivers/gpu/drm/i915/intel_pm.c
>> > +++ b/drivers/gpu/drm/i915/intel_pm.c
>> > @@ -2388,7 +2388,7 @@ static void hsw_compute_wm_parameters(struct drm_device *dev,
>> >                 pipe = intel_plane->pipe;
>> >                 p = &params[pipe];
>> >
>> > -               p->sprite_enabled = intel_plane->wm.enable;
>> > +               p->sprite_enabled = intel_plane->wm.enabled;
>> >                 p->spr_bytes_per_pixel = intel_plane->wm.bytes_per_pixel;
>> >                 p->spr_horiz_pixels = intel_plane->wm.horiz_pixels;
>> >
>> > @@ -2616,7 +2616,7 @@ static void haswell_update_wm(struct drm_device *dev)
>> >
>> >  static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
>> >                                      uint32_t sprite_width, int pixel_size,
>> > -                                    bool enable)
>> > +                                    bool enabled, bool scaled)
>> >  {
>> >         struct drm_plane *plane;
>> >
>> > @@ -2624,7 +2624,8 @@ static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
>> >                 struct intel_plane *intel_plane = to_intel_plane(plane);
>> >
>> >                 if (intel_plane->pipe == pipe) {
>> > -                       intel_plane->wm.enable = enable;
>> > +                       intel_plane->wm.enabled = enabled;
>> > +                       intel_plane->wm.scaled = scaled;
>> >                         intel_plane->wm.horiz_pixels = sprite_width + 1;
>> >                         intel_plane->wm.bytes_per_pixel = pixel_size;
>> >                         break;
>> > @@ -2712,7 +2713,7 @@ sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
>> >
>> >  static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>> >                                          uint32_t sprite_width, int pixel_size,
>> > -                                        bool enable)
>> > +                                        bool enable, bool scaled)
>> >  {
>> >         struct drm_i915_private *dev_priv = dev->dev_private;
>> >         int latency = SNB_READ_WM0_LATENCY() * 100;     /* In unit 0.1us */
>> > @@ -2835,13 +2836,13 @@ void intel_update_watermarks(struct drm_device *dev)
>> >
>> >  void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
>> >                                     uint32_t sprite_width, int pixel_size,
>> > -                                   bool enable)
>> > +                                   bool enable, bool scaled)
>> >  {
>> >         struct drm_i915_private *dev_priv = dev->dev_private;
>> >
>> >         if (dev_priv->display.update_sprite_wm)
>> >                 dev_priv->display.update_sprite_wm(dev, pipe, sprite_width,
>> > -                                                  pixel_size, enable);
>> > +                                                  pixel_size, enable, scaled);
>> >  }
>> >
>> >  static struct drm_i915_gem_object *
>> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> > index 1fa5612..5a1f3fd 100644
>> > --- a/drivers/gpu/drm/i915/intel_sprite.c
>> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> > @@ -114,7 +114,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_framebuffer *fb,
>> >         crtc_w--;
>> >         crtc_h--;
>> >
>> > -       intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
>> > +       intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
>> > +                                      src_w != crtc_w || src_h != crtc_h);
>> >
>> >         I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
>> >         I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
>> > @@ -268,7 +269,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>> >         crtc_w--;
>> >         crtc_h--;
>> >
>> > -       intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
>> > +       intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
>> > +                                      src_w != crtc_w || src_h != crtc_h);
>> >
>> >         /*
>> >          * IVB workaround: must disable low power watermarks for at least
>> > @@ -335,7 +337,7 @@ ivb_disable_plane(struct drm_plane *plane)
>> >
>> >         dev_priv->sprite_scaling_enabled &= ~(1 << pipe);
>> >
>> > -       intel_update_sprite_watermarks(dev, pipe, 0, 0, false);
>> > +       intel_update_sprite_watermarks(dev, pipe, 0, 0, false, false);
>> >
>> >         /* potentially re-enable LP watermarks */
>> >         if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)
>> > @@ -455,7 +457,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>> >         crtc_w--;
>> >         crtc_h--;
>> >
>> > -       intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
>> > +       intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
>> > +                                      src_w != crtc_w || src_h != crtc_h);
>> >
>> >         dvsscale = 0;
>> >         if (IS_GEN5(dev) || crtc_w != src_w || crtc_h != src_h)
>> > --
>> > 1.8.1.5
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Paulo Zanoni
>
> --
> Ville Syrjälä
> Intel OTC



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux