[Intel-gfx] [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping

[Laurent Pinchart]

Hi Ville,

On Monday, 6 November 2017 20:04:38 EET Ville Syrjälä wrote:
> 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 at 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 at vmware.com>
> >>> Cc: Sinclair Yeh <syeh at vmware.com>
> >>> Cc: Thomas Hellstrom <thellstrom at vmware.com>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >> 
> >> I kinda wonder whether we shouldn't push this down into the helper ...

I think that would be nice.

> > 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 suppose it's a mix of cargo-cult and the fact that adjusted_mode hints that 
it contains the timings that should be applied to the hardware. That's why I 
use it in rcar-du.

Are drivers allowed to adjust the hdisplay and vdisplay values though ? I 
thought unsupported values there had to be rejected with an error at atomic 
check time, not adjusted.

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

I'll review and test the rcar-du patch :-)

-- 
Regards,

Laurent Pinchart