From: Daniel Vetter <daniel@ffwll.ch>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: ander.conselvan.de.oliveira@intel.com, daniel.vetter@intel.com,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 06/20] drm/i915: Convert primary plane 16.16 values to regular ints
Date: Tue, 7 Apr 2015 10:43:51 +0200 [thread overview]
Message-ID: <20150407084351.GW6354@phenom.ffwll.local> (raw)
In-Reply-To: <20150402230322.GV28205@intel.com>
On Thu, Apr 02, 2015 at 04:03:22PM -0700, Matt Roper wrote:
> On Wed, Apr 01, 2015 at 07:59:35PM -0700, Chandra Konduru wrote:
> > This patch converts intel_plane_state->src rect from 16.16
> > values into regular ints.
> >
> > This approach aligns with sprite_plane_state->src rects
> > which are already in regular ints.
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
>
> You're not touching cursor state here, so I guess it stays in 16.16 form
> always? Does it need to be updated to behave the same way?
>
> Looking at our sprite code today, it treats intel_state->src as 16.16
> for most of the check function, then re-writes it as integer pixels near
> the end, which I guess matches the type of change you're doing here. I
> still find this pretty confusing that our structure is sometimes
> interpreted in one way and other times interpreted a different way.
>
> Personally I think it would be less error-prone if we just treated src
> as 16.16 always, but if you to keep the current logic which changes the
> meaning at the end of the check() stage, can you add some comments to
> struct intel_plane_state clarifying that?
Rewriting intel_plane_state won't work since on duplicate_state we'd need
to undo that again. That's a bit too brittle imo. I'd go with Matt's
suggestion to just use 16.16 everywhere.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-04-07 8:41 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-02 2:59 [PATCH 00/20] skylake display scalers Chandra Konduru
2015-04-02 2:59 ` [PATCH 01/20] drm/i915: Adding drm helper function drm_plane_from_index() Chandra Konduru
2015-04-02 23:01 ` Matt Roper
2015-04-02 2:59 ` [PATCH 02/20] drm/i915: Register definitions for skylake scalers Chandra Konduru
2015-04-02 2:59 ` [PATCH 03/20] drm/i915: skylake scaler structure definitions Chandra Konduru
2015-04-02 23:01 ` Matt Roper
2015-04-02 2:59 ` [PATCH 04/20] drm/i915: Initialize plane colorkey to NONE Chandra Konduru
2015-04-02 2:59 ` [PATCH 05/20] drm/i915: Initialize skylake scalers Chandra Konduru
2015-04-02 2:59 ` [PATCH 06/20] drm/i915: Convert primary plane 16.16 values to regular ints Chandra Konduru
2015-04-02 23:03 ` Matt Roper
2015-04-07 8:43 ` Daniel Vetter [this message]
2015-04-07 18:29 ` Konduru, Chandra
2015-04-07 18:45 ` Matt Roper
2015-04-07 19:02 ` Konduru, Chandra
2015-04-02 2:59 ` [PATCH 07/20] drm/i915: Dump scaler_state too as part of dumping crtc_state Chandra Konduru
2015-04-02 2:59 ` [PATCH 08/20] drm/i915: Helper function to update skylake scaling ratio Chandra Konduru
2015-04-02 23:03 ` Matt Roper
2015-04-02 2:59 ` [PATCH 09/20] drm/i915: Add helper function to update scaler_users in crtc_state Chandra Konduru
2015-04-02 23:04 ` Matt Roper
2015-04-02 2:59 ` [PATCH 10/20] drm/i915: Add atomic function to setup scalers scalers for a crtc Chandra Konduru
2015-04-02 23:04 ` Matt Roper
2015-04-06 4:44 ` Konduru, Chandra
2015-04-02 2:59 ` [PATCH 11/20] drm/i915: Helper function to detach a scaler from a plane or crtc Chandra Konduru
2015-04-02 23:04 ` Matt Roper
2015-04-02 2:59 ` [PATCH 12/20] drm/i915: Preserve scaler state when clearing crtc_state Chandra Konduru
2015-04-02 2:59 ` [PATCH 13/20] drm/i915: use current scaler state during readout_hw_state Chandra Konduru
2015-04-02 23:04 ` Matt Roper
2015-04-06 4:52 ` Konduru, Chandra
2015-04-02 2:59 ` [PATCH 14/20] drm/i915: Update scaling ratio as part of crtc_compute_config Chandra Konduru
2015-04-02 2:59 ` [PATCH 15/20] drm/i915: Ensure setting up scalers into staged crtc_state Chandra Konduru
2015-04-02 2:59 ` [PATCH 16/20] drm/i915: copy staged scaler state from drm state to crtc->config Chandra Konduru
2015-04-02 2:59 ` [PATCH 17/20] drm/i915: stage panel fitting scaler request for fixed mode panel Chandra Konduru
2015-04-02 2:59 ` [PATCH 18/20] drm/i915: Enable skylake panel fitting using skylake shared scalers Chandra Konduru
2015-04-02 2:59 ` [PATCH 19/20] drm/i915: Enable skylake primary plane scaling using " Chandra Konduru
2015-04-02 23:05 ` Matt Roper
2015-04-02 2:59 ` [PATCH 20/20] drm/i915: Enable skylake sprite " Chandra Konduru
2015-04-02 14:44 ` shuang.he
2015-04-02 17:20 ` Konduru, Chandra
2015-04-03 2:50 ` He, Shuang
2015-04-02 23:05 ` Matt Roper
2015-04-02 23:20 ` [PATCH 00/20] skylake display scalers Matt Roper
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=20150407084351.GW6354@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=ander.conselvan.de.oliveira@intel.com \
--cc=daniel.vetter@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=matthew.d.roper@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