| Message ID | 1443023547-19896-7-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 23, 2015 at 12:52:26PM -0300, Paulo Zanoni wrote: > The trick is not strictly necessary on SKL because the offset > registers allow more bits. But for FBC, doing this changes how the > hardware tracking works - it starts at the surface address we provide > - so there's a higher chance that the CRTC will be pointing to an area > of the frontbuffer that is actually being covered by the hardware > tracking mechanism. This fixes fbc-farfromfence on SKL. > > Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 24b8a72..d40ae71 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3031,6 +3031,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > int src_x = 0, src_y = 0, src_w = 0, src_h = 0; > int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; > int scaler_id = -1; > + int pixel_size; > > plane_state = to_intel_plane_state(plane->state); > > @@ -3079,6 +3080,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > src_h = intel_crtc->config->pipe_src_h; > } > > + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); > + intel_crtc->dspaddr_offset = intel_gen4_compute_page_offset(dev_priv, > + &x, &y, obj->tiling_mode, > + pixel_size, fb->pitches[0]); > + surf_addr += intel_crtc->dspaddr_offset; > + > if (intel_rotation_90_or_270(rotation)) { > /* stride = Surface height in tiles */ > tile_height = intel_tile_height(dev, fb->pixel_format, Tvrtko mind running this past your 90/270 rotation sanity tester? -Chris
On 09/23/2015 06:03 PM, Chris Wilson wrote: > On Wed, Sep 23, 2015 at 12:52:26PM -0300, Paulo Zanoni wrote: >> The trick is not strictly necessary on SKL because the offset >> registers allow more bits. But for FBC, doing this changes how the >> hardware tracking works - it starts at the surface address we provide >> - so there's a higher chance that the CRTC will be pointing to an area >> of the frontbuffer that is actually being covered by the hardware >> tracking mechanism. This fixes fbc-farfromfence on SKL. >> >> Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 24b8a72..d40ae71 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -3031,6 +3031,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, >> int src_x = 0, src_y = 0, src_w = 0, src_h = 0; >> int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; >> int scaler_id = -1; >> + int pixel_size; >> >> plane_state = to_intel_plane_state(plane->state); >> >> @@ -3079,6 +3080,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, >> src_h = intel_crtc->config->pipe_src_h; >> } >> >> + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); >> + intel_crtc->dspaddr_offset = intel_gen4_compute_page_offset(dev_priv, >> + &x, &y, obj->tiling_mode, >> + pixel_size, fb->pitches[0]); >> + surf_addr += intel_crtc->dspaddr_offset; >> + >> if (intel_rotation_90_or_270(rotation)) { >> /* stride = Surface height in tiles */ >> tile_height = intel_tile_height(dev, fb->pixel_format, > > Tvrtko mind running this past your 90/270 rotation sanity tester? You mean igt/kms_rotation_crc ? I don't have the background of what is this doing, but what jumps out at me is use of obj->tiling_mode which is not used for display tiling on skl+, and tile size calculations in intel_gen4_compute_page_offset seems to only support X tiling. The two together would make me say that it can't possibly work. :) Maybe Paolo can get quicker to running that igt since it sounds like he is already set up? Regards, Tvrtko
On Thu, Sep 24, 2015 at 10:55:17AM +0100, Tvrtko Ursulin wrote: > > On 09/23/2015 06:03 PM, Chris Wilson wrote: > >On Wed, Sep 23, 2015 at 12:52:26PM -0300, Paulo Zanoni wrote: > >>The trick is not strictly necessary on SKL because the offset > >>registers allow more bits. But for FBC, doing this changes how the > >>hardware tracking works - it starts at the surface address we provide > >>- so there's a higher chance that the CRTC will be pointing to an area > >>of the frontbuffer that is actually being covered by the hardware > >>tracking mechanism. This fixes fbc-farfromfence on SKL. > >> > >>Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence > >>Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > >>--- > >> drivers/gpu/drm/i915/intel_display.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >>index 24b8a72..d40ae71 100644 > >>--- a/drivers/gpu/drm/i915/intel_display.c > >>+++ b/drivers/gpu/drm/i915/intel_display.c > >>@@ -3031,6 +3031,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > >> int src_x = 0, src_y = 0, src_w = 0, src_h = 0; > >> int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; > >> int scaler_id = -1; > >>+ int pixel_size; > >> > >> plane_state = to_intel_plane_state(plane->state); > >> > >>@@ -3079,6 +3080,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > >> src_h = intel_crtc->config->pipe_src_h; > >> } > >> > >>+ pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); > >>+ intel_crtc->dspaddr_offset = intel_gen4_compute_page_offset(dev_priv, > >>+ &x, &y, obj->tiling_mode, > >>+ pixel_size, fb->pitches[0]); > >>+ surf_addr += intel_crtc->dspaddr_offset; > >>+ > >> if (intel_rotation_90_or_270(rotation)) { > >> /* stride = Surface height in tiles */ > >> tile_height = intel_tile_height(dev, fb->pixel_format, > > > >Tvrtko mind running this past your 90/270 rotation sanity tester? > > You mean igt/kms_rotation_crc ? I actually meant you! :) -Chris
On Wed, Sep 23, 2015 at 12:52:26PM -0300, Paulo Zanoni wrote: > The trick is not strictly necessary on SKL because the offset > registers allow more bits. But for FBC, doing this changes how the > hardware tracking works - it starts at the surface address we provide > - so there's a higher chance that the CRTC will be pointing to an area > of the frontbuffer that is actually being covered by the hardware > tracking mechanism. This fixes fbc-farfromfence on SKL. > > Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 24b8a72..d40ae71 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3031,6 +3031,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > int src_x = 0, src_y = 0, src_w = 0, src_h = 0; > int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; > int scaler_id = -1; > + int pixel_size; > > plane_state = to_intel_plane_state(plane->state); > > @@ -3079,6 +3080,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > src_h = intel_crtc->config->pipe_src_h; > } > > + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); > + intel_crtc->dspaddr_offset = intel_gen4_compute_page_offset(dev_priv, > + &x, &y, obj->tiling_mode, > + pixel_size, fb->pitches[0]); > + surf_addr += intel_crtc->dspaddr_offset; It's not that simple. I have a branch that tries to make the required changes to make it work properly: git://github.com/vsyrjala/linux.git tile_size > + > if (intel_rotation_90_or_270(rotation)) { > /* stride = Surface height in tiles */ > tile_height = intel_tile_height(dev, fb->pixel_format, > -- > 2.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
What's the next step on this patch? Gavin Hindman Senior Program Manager SSG/OTC – Open Source Technology Center -----Original Message----- From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Ville Syrjälä Sent: Thursday, September 24, 2015 10:10 AM To: Zanoni, Paulo R <paulo.r.zanoni@intel.com> Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 6/7] drm/i915: use compute_page_offset() on SKL too On Wed, Sep 23, 2015 at 12:52:26PM -0300, Paulo Zanoni wrote: > The trick is not strictly necessary on SKL because the offset > registers allow more bits. But for FBC, doing this changes how the > hardware tracking works - it starts at the surface address we provide > - so there's a higher chance that the CRTC will be pointing to an area > of the frontbuffer that is actually being covered by the hardware > tracking mechanism. This fixes fbc-farfromfence on SKL. > > Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 24b8a72..d40ae71 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3031,6 +3031,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > int src_x = 0, src_y = 0, src_w = 0, src_h = 0; > int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; > int scaler_id = -1; > + int pixel_size; > > plane_state = to_intel_plane_state(plane->state); > > @@ -3079,6 +3080,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > src_h = intel_crtc->config->pipe_src_h; > } > > + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); > + intel_crtc->dspaddr_offset = intel_gen4_compute_page_offset(dev_priv, > + &x, &y, obj->tiling_mode, > + pixel_size, fb->pitches[0]); > + surf_addr += intel_crtc->dspaddr_offset; It's not that simple. I have a branch that tries to make the required changes to make it work properly: git://github.com/vsyrjala/linux.git tile_size > + > if (intel_rotation_90_or_270(rotation)) { > /* stride = Surface height in tiles */ > tile_height = intel_tile_height(dev, fb->pixel_format, > -- > 2.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Oct 12, 2015 at 06:19:52PM +0000, Hindman, Gavin wrote: > What's the next step on this patch? I think my branch is nearing usable state. But I haven't actually tested on SKL yet, which means not tested 90/270 rotation either. That's pretty much the next thing on my list. > > Gavin Hindman > Senior Program Manager > SSG/OTC – Open Source Technology Center > > -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Ville Syrjälä > Sent: Thursday, September 24, 2015 10:10 AM > To: Zanoni, Paulo R <paulo.r.zanoni@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 6/7] drm/i915: use compute_page_offset() on SKL too > > On Wed, Sep 23, 2015 at 12:52:26PM -0300, Paulo Zanoni wrote: > > The trick is not strictly necessary on SKL because the offset > > registers allow more bits. But for FBC, doing this changes how the > > hardware tracking works - it starts at the surface address we provide > > - so there's a higher chance that the CRTC will be pointing to an area > > of the frontbuffer that is actually being covered by the hardware > > tracking mechanism. This fixes fbc-farfromfence on SKL. > > > > Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 24b8a72..d40ae71 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3031,6 +3031,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > > int src_x = 0, src_y = 0, src_w = 0, src_h = 0; > > int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; > > int scaler_id = -1; > > + int pixel_size; > > > > plane_state = to_intel_plane_state(plane->state); > > > > @@ -3079,6 +3080,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > > src_h = intel_crtc->config->pipe_src_h; > > } > > > > + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); > > + intel_crtc->dspaddr_offset = intel_gen4_compute_page_offset(dev_priv, > > + &x, &y, obj->tiling_mode, > > + pixel_size, fb->pitches[0]); > > + surf_addr += intel_crtc->dspaddr_offset; > > It's not that simple. I have a branch that tries to make the required changes to make it work properly: > git://github.com/vsyrjala/linux.git tile_size > > > + > > if (intel_rotation_90_or_270(rotation)) { > > /* stride = Surface height in tiles */ > > tile_height = intel_tile_height(dev, fb->pixel_format, > > -- > > 2.5.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 24b8a72..d40ae71 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3031,6 +3031,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, int src_x = 0, src_y = 0, src_w = 0, src_h = 0; int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0; int scaler_id = -1; + int pixel_size; plane_state = to_intel_plane_state(plane->state); @@ -3079,6 +3080,12 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, src_h = intel_crtc->config->pipe_src_h; } + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); + intel_crtc->dspaddr_offset = intel_gen4_compute_page_offset(dev_priv, + &x, &y, obj->tiling_mode, + pixel_size, fb->pitches[0]); + surf_addr += intel_crtc->dspaddr_offset; + if (intel_rotation_90_or_270(rotation)) { /* stride = Surface height in tiles */ tile_height = intel_tile_height(dev, fb->pixel_format,
The trick is not strictly necessary on SKL because the offset registers allow more bits. But for FBC, doing this changes how the hardware tracking works - it starts at the surface address we provide - so there's a higher chance that the CRTC will be pointing to an area of the frontbuffer that is actually being covered by the hardware tracking mechanism. This fixes fbc-farfromfence on SKL. Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 7 +++++++ 1 file changed, 7 insertions(+)