| Message ID | 1426398946-5900-5-git-send-email-chandra.konduru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > 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 >
> -----Original Message----- > From: Roper, Matthew D > Sent: Tuesday, March 17, 2015 9:13 AM > To: Konduru, Chandra > Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander > Subject: Re: [PATCH 04/21] drm/i915: skylake scaler structure definitions > > 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@intel.com> > > --- > > 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. I put them per scaler basis for future scalability, but can make them as one copy. It has some cascading effect on other patches so send out this one and other impacted ones. > > > + 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. num_scalers isn't symmetric across pipes, so we need maintain it per crtc. > > > + 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. I looked at other variables carrying bits, for example, disable_pipes, prepare_pipes, modeset_pipes, disabled_planes etc. and all are using just like above. And followed the same. > > > + > > + /* 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
On Tue, Mar 17, 2015 at 09:20:11PM +0000, Konduru, Chandra wrote: > > > > -----Original Message----- > > From: Roper, Matthew D > > Sent: Tuesday, March 17, 2015 9:13 AM > > To: Konduru, Chandra > > Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander > > Subject: Re: [PATCH 04/21] drm/i915: skylake scaler structure definitions > > > > 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@intel.com> > > > --- > > > 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. > > I put them per scaler basis for future scalability, but can make them as one copy. > It has some cascading effect on other patches so send out this one and other > impacted ones. In my experience adding flexibility in the design for which we don't yet have a clear use-case established is a tricky design mistake: It always sounds good and very often turns out to be a costly endeavor. I much prefer to be as lazy as possible in code design and if there's no need yet to not write code. Also, can't we recompute these limits at runtime? I can't quickly check since the code is in some other patches, but iirc that would resolve some of the issues I've pointed out later on with keeping these in sync. I think as a design rule atomic state structures should be constrained to the input values (we need them by design of the atomic interfaces) and the computed values we want to put into the hw. Intermediate results, especially for constraint checking just complicate all the state handling. There are some exceptions (e.g. pll computation where we have intermediate dotclock values), but adding state to the permanent state structures has costs. Imo better to avoid it. -Daniel
On Wed, Mar 18, 2015 at 09:00:20AM +0100, Daniel Vetter wrote: > On Tue, Mar 17, 2015 at 09:20:11PM +0000, Konduru, Chandra wrote: > > > > > > > -----Original Message----- > > > From: Roper, Matthew D > > > Sent: Tuesday, March 17, 2015 9:13 AM > > > To: Konduru, Chandra > > > Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander > > > Subject: Re: [PATCH 04/21] drm/i915: skylake scaler structure definitions > > > > > > 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@intel.com> > > > > --- > > > > 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. > > > > I put them per scaler basis for future scalability, but can make them as one copy. > > It has some cascading effect on other patches so send out this one and other > > impacted ones. > > In my experience adding flexibility in the design for which we don't yet > have a clear use-case established is a tricky design mistake: It always > sounds good and very often turns out to be a costly endeavor. I much > prefer to be as lazy as possible in code design and if there's no need yet > to not write code. > > Also, can't we recompute these limits at runtime? I can't quickly check > since the code is in some other patches, but iirc that would resolve some > of the issues I've pointed out later on with keeping these in sync. > > I think as a design rule atomic state structures should be constrained to > the input values (we need them by design of the atomic interfaces) and the > computed values we want to put into the hw. Intermediate results, > especially for constraint checking just complicate all the state handling. > There are some exceptions (e.g. pll computation where we have intermediate > dotclock values), but adding state to the permanent state structures has > costs. Imo better to avoid it. Ok forget about all this, somehow I got really confused with the split between intel_scaler and intel_scaler_state. Dunno why but I thought some of the limits are in scaler_state and need to be recomputed. I'll follow-up with new comments. -Daniel
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. + */ + 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; + 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; + + /* 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 {
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@intel.com> --- drivers/gpu/drm/i915/intel_drv.h | 99 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+)