On Wed, Dec 18, 2013 at 01:41:21PM +0530, Vandana Kannan wrote:
> On Dec-17-2013 5:58 PM, Chris Wilson wrote:
> > On Tue, Dec 17, 2013 at 10:58:24AM +0530, Vandana Kannan wrote:
> >> From: Pradeep Bhat <pradeep.bhat@xxxxxxxxx>
> >>
> >> This patch and finds out the lowest refresh rate supported for the resolution
> >> same as the fixed_mode, based on the implementaion find_panel_downclock.
> >> It also checks the VBT fields to see if panel supports seamless DRRS or not.
> >> Based on above data it marks whether eDP panel supports seamless DRRS or not.
> >> This information is needed for supporting seamless DRRS switch for
> >> certain power saving usecases. This patch is tested by enabling the DRM logs
> >> and user should see whether Seamless DRRS is supported or not.
> >>
> >> Signed-off-by: Pradeep Bhat <pradeep.bhat@xxxxxxxxx>
> >> Signed-off-by: Vandana Kannan <vandana.kannan@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> >> drivers/gpu/drm/i915/intel_dp.c | 47 ++++++++++++++++++++++++++++++++++++++
> >> drivers/gpu/drm/i915/intel_drv.h | 29 +++++++++++++++++++++++
> >> 3 files changed, 78 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 02e11dc..c9bca16 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -1462,8 +1462,10 @@ typedef struct drm_i915_private {
> >> /* Reclocking support */
> >> bool render_reclock_avail;
> >> bool lvds_downclock_avail;
> >> + bool edp_downclock_avail;
> >> /* indicates the reduced downclock for LVDS*/
> >> int lvds_downclock;
> >> + int edp_downclock;
> >> u16 orig_clock;
> >
> > Do any machines have both edp and lvds? Shouldn't this be a part of the
> > panel state?
> >
> If there is a machine having both edp and lvds, then edp takes higher
> priority. edp_downclock_avail and edp_downclock were added here
> following the existing code having lvds_downclock_avail and
> lvds_downclock here. If required, edp_downclock_avail and edp_downclock
> can be moved to intel_panel structure. Kindly let us know.
And we can consolidate both into intel_panel.
> >>
> >> +/**
> >> + * This enum is used to indicate the DRRS support type.
> >> + * The values of the enum map 1-to-1 with the values from VBT.
> >> + */
> >> +enum edp_panel_type {
> >> + DRRS_NOT_SUPPORTED = -1,
> >> + STATIC_DRRS_SUPPORT = 0,
> >> + SEAMLESS_DRRS_SUPPORT = 2
> >> +};
> >> +/**
> >> + * HIGH_RR is the highest eDP panel refresh rate read from EDID
> >> + * LOW_RR is the lowest eDP panel refresh rate found from EDID
> >> + * parsing for same resolution.
> >> + */
> >> +enum edp_drrs_refresh_rate_type {
> >> + DRRS_HIGH_RR,
> >> + DRRS_LOW_RR,
> >> + DRRS_MAX_RR, /* RR count */
> >> +};
> >> +/**
> >> + * The drrs_info struct will represent the DRRS feature for eDP
> >> + * panel.
> >> + */
> >> +struct drrs_info {
> >> + int is_drrs_supported;
> >> + int drrs_refresh_rate_type;
> >
> > So what was the point of the enums again? Are you purposely trying to
> > disable gcc and sparse's type-safety?
> >
> The enum edp_panel_type is required to check DRRS capability of the
> panel before performing any enabling. We will look into an
> implementation which can do without edp_drrs_refresh_rate_type.
All I am saying is have enum, use enum. It improves type safety.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
