dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: kernel@pengutronix.de, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v4] drm/imx: ipuv3-plane: use drm_plane_helper_check_state, clipped coordinates
Date: Tue, 07 Feb 2017 09:51:50 +0100	[thread overview]
Message-ID: <1486457510.2277.8.camel@pengutronix.de> (raw)
In-Reply-To: <20170203132056.GJ31595@intel.com>

On Fri, 2017-02-03 at 15:20 +0200, Ville Syrjälä wrote:
> On Fri, Feb 03, 2017 at 10:31:39AM +0100, Philipp Zabel wrote:
> > Use drm_plane_helper_check_state to clip raw user coordinates to crtc
> > bounds. This checks for full plane coverage and scaling already, so
> > we can drop some custom checks. Use the clipped coordinates everywhere.
> > 
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> > Changes since v3:
> >  - Disallow target frames that start offscreen (state->crtc_x/y < 0), to avoid
> >    confusing userspace: due to the necessary line start address alignment, we
> >    could only support very specific negative values for crtc_x/y, depending on
> >    the pixel format.
> 
> That's really no different to the user specifying non-zero src
> coordinates. So I'm wondering what's the point of special casing
> crtc coordinates this way because you'll need to check the src
> coordinates anyway.

User expectations.

For the src_x/y / src.x1/y1 source coordinates we have format specific
alignment requirements due to limitations of the DMA unit, there is no
way around it.
For the crtc_x/y plane coordinates there are no alignment requirements
at all, the partial plane can be positioned at any integer x/y position.
If we allow to simulate a partially offscreen plane at negative
positions by clipping and setting src.x1/y1, suddenly the crtc_x/y have
alignment requirements, but only if the values are negative.
I assume that would rather confuse any user space application that tries
to reason about which crtc_x/y values are valid, so it's probably better
to disallow it altogether.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2017-02-07  8:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-03  9:31 [PATCH v4] drm/imx: ipuv3-plane: use drm_plane_helper_check_state, clipped coordinates Philipp Zabel
2017-02-03 13:20 ` Ville Syrjälä
2017-02-07  8:51   ` Philipp Zabel [this message]
2017-02-07 11:19     ` Ville Syrjälä
2017-02-07 11:49       ` Philipp Zabel

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=1486457510.2277.8.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kernel@pengutronix.de \
    --cc=ville.syrjala@linux.intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox