On Sat, Mar 14, 2015 at 10:55:29PM -0700, Chandra Konduru wrote:
> skylake scaler structure definitions. scalers live in crtc_state as
> they are pipe resources. They can be used either as plane scaler or
> panel fitter.
>
> scaler assigned to either plane (for plane scaling) or crtc (for panel
> fitting) is saved in scaler_id in plane_state or crtc_state respectively.
>
> scaler_id is used instead of scaler pointer in plane or crtc state
> to avoid updating scaler pointer everytime a new crtc_state is created.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 99 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3f7d05e..d9a3b64 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -256,6 +256,35 @@ struct intel_plane_state {
> * enable/disable the primary plane
> */
> bool hides_primary;
> +
> + /*
> + * scaler_id
> + * = -1 : not using a scaler
> + * >= 0 : using a scalers
> + *
> + * plane requiring a scaler:
> + * - During check_plane, its bit is set in
> + * crtc_state->scaler_state.scaler_users by calling helper function
> + * update_scaler_users.
> + * - scaler_id indicates the scaler it got assigned.
> + *
> + * plane doesn't require a scaler:
> + * - this can happen when scaling is no more required or plane simply
> + * got disabled.
> + * - During check_plane, corresponding bit is reset in
> + * crtc_state->scaler_state.scaler_users by calling helper function
> + * update_scaler_users.
> + *
> + * There are two scenarios:
> + * 1. the freed up scaler is assigned to crtc or some other plane
> + * In this case, as part of plane programming scaler_id will be set
> + * to -1 using helper function detach_scalers
> + * 2. the freed up scaler is not assigned to anyone
> + * In this case, as part of plane programming scaler registers will
> + * be reset and scaler_id will also be reset to -1 using the same
> + * helper function detach_scalers
> + */
> + int scaler_id;
> };
>
> struct intel_initial_plane_config {
> @@ -265,6 +294,74 @@ struct intel_initial_plane_config {
> u32 base;
> };
>
> +struct intel_scaler {
> + int id;
> + int in_use;
> + uint32_t mode;
> + uint32_t filter;
> +
> + /*
> + * Supported scaling ratio is represented as a range in [min max]
> + * variables. This range covers both up and downscaling
> + * where scaling ratio = (dst * 100)/src.
> + * In above range any value:
> + * < 100 represents downscaling coverage
> + * > 100 represents upscaling coverage
> + * = 100 represents no-scaling (i.e., 1:1)
> + * e.g., a min value = 50 means -> supports upto 50% of original image
> + * a max value = 200 means -> supports upto 200% of original image
> + *
> + * if incoming flip requires scaling in the supported [min max] range
> + * then requested scaling will be performed.
> + */
I've only skimmed a little ahead in the series, so I might have missed
something, but do we really need to track these on a per-scaler basis?
When you use the values here, I think you're always pulling the values
from scaler[0] unconditionally from what I saw so duplicating the values
for each scaler doesn't really help us.
Is it possible to keep just one copy of these min/max values? On SKL
all of our scalers are homogeneous, so it doesn't feel like there's too
much value to duplicating these values. If we have a future platform
with heterogeneous scalers, it seems like we can figure out how to
handle that appropriately if/when we get there.
> + uint32_t min_hsr;
> + uint32_t max_hsr;
> + uint32_t min_vsr;
> + uint32_t max_vsr;
> + uint32_t min_hvsr;
> + uint32_t max_hvsr;
> +
> + uint32_t min_src_w;
> + uint32_t max_src_w;
> + uint32_t min_src_h;
> + uint32_t max_src_h;
> + uint32_t min_dst_w;
> + uint32_t max_dst_w;
> + uint32_t min_dst_h;
> + uint32_t max_dst_h;
> +};
> +
> +struct intel_crtc_scaler_state {
> +#define INTEL_MAX_SCALERS 2
> +#define SKL_NUM_SCALERS INTEL_MAX_SCALERS
> + /* scalers available on this crtc */
> + int num_scalers;
Maybe add .num_scalers to the device_info struct? I know it doesn't
make much of a difference, but it feels cleaner to have immutable traits
of the hardware in device_info or even the base intel_crtc structure and
leave the state variable for tracking things that can change at runtime.
> + struct intel_scaler scalers[INTEL_MAX_SCALERS];
> +
> + /*
> + * scaler_users: keeps track of users requesting scalers on this crtc.
> + *
> + * If a bit is set, a user is using a scaler.
> + * Here user can be a plane or crtc as defined below:
> + * bits 0-30 - plane (bit position is index from drm_plane_index)
> + * bit 31 - crtc
> + *
> + * Instead of creating a new index to cover planes and crtc, using
> + * existing drm_plane_index for planes which is well less than 31
> + * planes and bit 31 for crtc. This should be fine to cover all
> + * our platforms.
> + *
> + * intel_atomic_setup_scalers will setup available scalers to users
> + * requesting scalers. It will gracefully fail if request exceeds
> + * avilability.
> + */
> +#define SKL_CRTC_INDEX 31
> + unsigned scaler_users;
Might be slightly preferable to use a type with a specific size when
creating a bitmask? I.e., uint32_t or uint64_t, just to be explicit.
> +
> + /* scaler used by crtc for panel fitting purpose */
> + int scaler_id;
> +};
> +
> struct intel_crtc_state {
> struct drm_crtc_state base;
>
> @@ -391,6 +488,8 @@ struct intel_crtc_state {
>
> bool dp_encoder_is_mst;
> int pbn;
> +
> + struct intel_crtc_scaler_state scaler_state;
> };
>
> struct intel_pipe_wm {
> --
> 1.7.9.5
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
