All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org,
	VMware Graphics <linux-graphics-maintainer@vmware.com>,
	Thomas Hellstrom <thellstrom@vmware.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping
Date: Mon, 6 Nov 2017 20:04:38 +0200	[thread overview]
Message-ID: <20171106180437.GM10981@intel.com> (raw)
In-Reply-To: <20171102131930.GL10981@intel.com>

On Thu, Nov 02, 2017 at 03:19:30PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> > On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Try to fix the code to actually clip the plane to the crtc bounds
> > > instead of the user provided crtc coordinates (which would be a no-op
> > > since those are exactly the coordinates before clipping).
> > > 
> > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I kinda wonder whether we shouldn't push this down into the helper ...
> 
> Would require quite going through all drivers making sure they are
> happy with using the adjusted_mode.crtc_ timings. I think most drivers
> use the other adjusted_mode timings currently, and some even use the
> user mode timings (which could be an actual bug).

Let me take that back. What we want is to clip to the user mode
timings. Stereo 3D needs the special treatment from
drm_mode_get_hv_timing(). I'm getting confused by all these
different timings we have all over the place.

I think for i915 all we would need is to change the double wide/dual
link adjustent for pipe_src_w to simply reject odd widths instead.
That would guarantee that the user mode timings match the pipe_src_w/h
100%.

For the other driver we'd need to figure out why they're using
adjusted_mode timings for clipping. I guess that's just a mistake,
which I repeated here with vmwgfx after getting confused by looking
at the other drivers.

I guess I just volunteered myself to do this. Just needs plenty of
acks from driver maintainers I suppose.

> 
> > 
> > Either way, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > > ---
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
> > >  1 file changed, 13 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > index 515b67783a41..efa41c086198 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
> > >  int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> > >  				      struct drm_plane_state *state)
> > >  {
> > > +	struct drm_crtc_state *crtc_state = NULL;
> > >  	struct drm_framebuffer *new_fb = state->fb;
> > > -	struct drm_rect clip = {
> > > -		.x1 = state->crtc_x,
> > > -		.y1 = state->crtc_y,
> > > -		.x2 = state->crtc_x + state->crtc_w,
> > > -		.y2 = state->crtc_y + state->crtc_h,
> > > -	};
> > > +	struct drm_rect clip = {};
> > >  	int ret;
> > >  
> > > -	ret = drm_plane_helper_check_state(state, &clip,
> > > -					    DRM_PLANE_HELPER_NO_SCALING,
> > > -					    DRM_PLANE_HELPER_NO_SCALING,
> > > -					    false, true);
> > > +	if (state->crtc)
> > > +		crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
> > >  
> > > +	if (crtc_state && crtc_state->enable) {
> > > +		clip.x2 = crtc_state->adjusted_mode.hdisplay;
> > > +		clip.y2 = crtc_state->adjusted_mode.vdisplay;
> > > +	}
> > > +
> > > +	ret = drm_plane_helper_check_state(state, &clip,
> > > +					   DRM_PLANE_HELPER_NO_SCALING,
> > > +					   DRM_PLANE_HELPER_NO_SCALING,
> > > +					   false, true);
> > >  
> > >  	if (!ret && new_fb) {
> > >  		struct drm_crtc *crtc = state->crtc;
> > > -- 
> > > 2.13.6
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2017-11-06 18:04 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
2017-11-01 18:29 ` [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check Ville Syrjala
2017-11-02 10:04   ` Daniel Vetter
2017-11-23  9:54   ` Laurent Pinchart
2017-11-01 18:29 ` [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state() Ville Syrjala
2017-11-02 10:06   ` Daniel Vetter
2017-11-23  9:54   ` Laurent Pinchart
2017-11-01 18:29 ` [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping Ville Syrjala
2017-11-02 10:12   ` Daniel Vetter
2017-11-02 13:19     ` Ville Syrjälä
2017-11-06 18:04       ` Ville Syrjälä [this message]
2017-11-07 12:30         ` Daniel Vetter
2017-11-23  9:46         ` [Intel-gfx] " Laurent Pinchart
2017-11-01 18:29 ` [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state() Ville Syrjala
2017-11-01 20:15   ` [PATCH v2 " Ville Syrjala
2017-11-02 10:15     ` Daniel Vetter
2017-11-23  9:52   ` [PATCH " Laurent Pinchart
2017-11-01 18:29 ` [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c Ville Syrjala
2017-11-01 20:16   ` [PATCH v2 " Ville Syrjala
2017-11-02 10:16     ` Daniel Vetter
2017-11-23  9:53   ` [PATCH " Laurent Pinchart
2017-11-01 19:46 ` ✗ Fi.CI.BAT: failure for drm: drm_plane_helper_check_state() related stuff Patchwork
2017-11-01 20:10   ` Ville Syrjälä
2017-11-01 21:02 ` ✓ Fi.CI.BAT: success for drm: drm_plane_helper_check_state() related stuff (rev3) Patchwork
2017-11-01 21:52 ` ✓ Fi.CI.IGT: " Patchwork
2017-11-10 21:26 ` [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Sinclair Yeh
2017-11-10 21:42   ` Ville Syrjälä
2017-11-20  7:34     ` Daniel Vetter
2017-11-20 17:32       ` Sinclair Yeh
2017-11-20 19:36         ` Ville Syrjälä
2017-11-23  9:56           ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171106180437.GM10981@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-graphics-maintainer@vmware.com \
    --cc=thellstrom@vmware.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.