Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL
On Tue, Mar 05, 2019 at 01:07:34PM -0800, Lucas De Marchi wrote: > On Tue, Mar 05, 2019 at 03:23:48PM +0200, Jani Nikula wrote: > >On Mon, 04 Mar 2019, Jani Nikula wrote: > >> On Mon, 04 Mar 2019, Ville Syrjälä wrote: > >>> On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote: > This register was placed in the middle of the PP_STATUS definition. Move > it down together with PP_CONTROL and fix the aligment of the bit > definition (as per documentation it should be 2 spaces instead of 1). > > Signed-off-by: Lucas De Marchi > --- > drivers/gpu/drm/i915/i915_reg.h | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index c9b868347481..bbbc0649a180 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4692,17 +4692,6 @@ enum { > #define _PP_STATUS 0x61200 > #define PP_STATUS(pps_idx) _MMIO_PPS(pps_idx, _PP_STATUS) > #define PP_ON (1 << 31) > - > -#define _PP_CONTROL_1 0xc7204 > -#define _PP_CONTROL_2 0xc7304 > -#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? > _PP_CONTROL_1 : \ > - _PP_CONTROL_2) > -#define POWER_CYCLE_DELAY_MASK (0x1f << 4) > -#define POWER_CYCLE_DELAY_SHIFT4 > -#define VDD_OVERRIDE_FORCE (1 << 3) > -#define BACKLIGHT_ENABLE (1 << 2) > -#define PWR_DOWN_ON_RESET (1 << 1) > -#define PWR_STATE_TARGET (1 << 0) > /* > * Indicates that all dependencies of the panel are on: > * > @@ -4728,6 +4717,17 @@ enum { > #define PP_SEQUENCE_STATE_ON_S1_3 (0xb << 0) > #define PP_SEQUENCE_STATE_RESET (0xf << 0) > > +#define _PP_CONTROL_1 0xc7204 > +#define _PP_CONTROL_2 0xc7304 > +#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? > _PP_CONTROL_1 : \ > + _PP_CONTROL_2) > +#define POWER_CYCLE_DELAY_MASK(0x1f << 4) > +#define POWER_CYCLE_DELAY_SHIFT 4 > +#define VDD_OVERRIDE_FORCE(1 << 3) > +#define BACKLIGHT_ENABLE (1 << 2) > +#define PWR_DOWN_ON_RESET (1 << 1) > +#define PWR_STATE_TARGET (1 << 0) > >>> > >>> This entire register looks 100% redundant. Just nuke the whole thing? > >> > >> Needed in the future? > > > >D'oh, missed the PPS base thing. Nuke it. > > But ICP_PP_CONTROL() is also unused. Should I nuke it as well? Yes. -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL
On Tue, Mar 05, 2019 at 03:23:48PM +0200, Jani Nikula wrote: On Mon, 04 Mar 2019, Jani Nikula wrote: On Mon, 04 Mar 2019, Ville Syrjälä wrote: On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote: This register was placed in the middle of the PP_STATUS definition. Move it down together with PP_CONTROL and fix the aligment of the bit definition (as per documentation it should be 2 spaces instead of 1). Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/i915/i915_reg.h | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c9b868347481..bbbc0649a180 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4692,17 +4692,6 @@ enum { #define _PP_STATUS 0x61200 #define PP_STATUS(pps_idx) _MMIO_PPS(pps_idx, _PP_STATUS) #define PP_ON(1 << 31) - -#define _PP_CONTROL_1 0xc7204 -#define _PP_CONTROL_2 0xc7304 -#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \ - _PP_CONTROL_2) -#define POWER_CYCLE_DELAY_MASK(0x1f << 4) -#define POWER_CYCLE_DELAY_SHIFT 4 -#define VDD_OVERRIDE_FORCE(1 << 3) -#define BACKLIGHT_ENABLE (1 << 2) -#define PWR_DOWN_ON_RESET (1 << 1) -#define PWR_STATE_TARGET (1 << 0) /* * Indicates that all dependencies of the panel are on: * @@ -4728,6 +4717,17 @@ enum { #define PP_SEQUENCE_STATE_ON_S1_3(0xb << 0) #define PP_SEQUENCE_STATE_RESET (0xf << 0) +#define _PP_CONTROL_1 0xc7204 +#define _PP_CONTROL_2 0xc7304 +#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \ + _PP_CONTROL_2) +#define POWER_CYCLE_DELAY_MASK (0x1f << 4) +#define POWER_CYCLE_DELAY_SHIFT 4 +#define VDD_OVERRIDE_FORCE (1 << 3) +#define BACKLIGHT_ENABLE (1 << 2) +#define PWR_DOWN_ON_RESET(1 << 1) +#define PWR_STATE_TARGET (1 << 0) This entire register looks 100% redundant. Just nuke the whole thing? Needed in the future? D'oh, missed the PPS base thing. Nuke it. But ICP_PP_CONTROL() is also unused. Should I nuke it as well? Lucas De Marchi BR, Jani. BR, Jani. + #define _PP_CONTROL0x61204 #define PP_CONTROL(pps_idx)_MMIO_PPS(pps_idx, _PP_CONTROL) #define PANEL_UNLOCK_REGS (0xabcd << 16) -- 2.20.1 -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL
On Mon, 04 Mar 2019, Jani Nikula wrote: > On Mon, 04 Mar 2019, Ville Syrjälä wrote: >> On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote: >>> This register was placed in the middle of the PP_STATUS definition. Move >>> it down together with PP_CONTROL and fix the aligment of the bit >>> definition (as per documentation it should be 2 spaces instead of 1). >>> >>> Signed-off-by: Lucas De Marchi >>> --- >>> drivers/gpu/drm/i915/i915_reg.h | 22 +++--- >>> 1 file changed, 11 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h >>> b/drivers/gpu/drm/i915/i915_reg.h >>> index c9b868347481..bbbc0649a180 100644 >>> --- a/drivers/gpu/drm/i915/i915_reg.h >>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>> @@ -4692,17 +4692,6 @@ enum { >>> #define _PP_STATUS 0x61200 >>> #define PP_STATUS(pps_idx) _MMIO_PPS(pps_idx, _PP_STATUS) >>> #define PP_ON(1 << 31) >>> - >>> -#define _PP_CONTROL_1 0xc7204 >>> -#define _PP_CONTROL_2 0xc7304 >>> -#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \ >>> - _PP_CONTROL_2) >>> -#define POWER_CYCLE_DELAY_MASK(0x1f << 4) >>> -#define POWER_CYCLE_DELAY_SHIFT 4 >>> -#define VDD_OVERRIDE_FORCE(1 << 3) >>> -#define BACKLIGHT_ENABLE (1 << 2) >>> -#define PWR_DOWN_ON_RESET (1 << 1) >>> -#define PWR_STATE_TARGET (1 << 0) >>> /* >>> * Indicates that all dependencies of the panel are on: >>> * >>> @@ -4728,6 +4717,17 @@ enum { >>> #define PP_SEQUENCE_STATE_ON_S1_3(0xb << 0) >>> #define PP_SEQUENCE_STATE_RESET (0xf << 0) >>> >>> +#define _PP_CONTROL_1 0xc7204 >>> +#define _PP_CONTROL_2 0xc7304 >>> +#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \ >>> + _PP_CONTROL_2) >>> +#define POWER_CYCLE_DELAY_MASK (0x1f << 4) >>> +#define POWER_CYCLE_DELAY_SHIFT 4 >>> +#define VDD_OVERRIDE_FORCE (1 << 3) >>> +#define BACKLIGHT_ENABLE (1 << 2) >>> +#define PWR_DOWN_ON_RESET(1 << 1) >>> +#define PWR_STATE_TARGET (1 << 0) >> >> This entire register looks 100% redundant. Just nuke the whole thing? > > Needed in the future? D'oh, missed the PPS base thing. Nuke it. BR, Jani. > > BR, > Jani. > >> >>> + >>> #define _PP_CONTROL0x61204 >>> #define PP_CONTROL(pps_idx)_MMIO_PPS(pps_idx, _PP_CONTROL) >>> #define PANEL_UNLOCK_REGS (0xabcd << 16) >>> -- >>> 2.20.1 -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL
On Mon, 04 Mar 2019, Ville Syrjälä wrote: > On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote: >> This register was placed in the middle of the PP_STATUS definition. Move >> it down together with PP_CONTROL and fix the aligment of the bit >> definition (as per documentation it should be 2 spaces instead of 1). >> >> Signed-off-by: Lucas De Marchi >> --- >> drivers/gpu/drm/i915/i915_reg.h | 22 +++--- >> 1 file changed, 11 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index c9b868347481..bbbc0649a180 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -4692,17 +4692,6 @@ enum { >> #define _PP_STATUS 0x61200 >> #define PP_STATUS(pps_idx) _MMIO_PPS(pps_idx, _PP_STATUS) >> #define PP_ON (1 << 31) >> - >> -#define _PP_CONTROL_1 0xc7204 >> -#define _PP_CONTROL_2 0xc7304 >> -#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \ >> - _PP_CONTROL_2) >> -#define POWER_CYCLE_DELAY_MASK (0x1f << 4) >> -#define POWER_CYCLE_DELAY_SHIFT4 >> -#define VDD_OVERRIDE_FORCE (1 << 3) >> -#define BACKLIGHT_ENABLE (1 << 2) >> -#define PWR_DOWN_ON_RESET (1 << 1) >> -#define PWR_STATE_TARGET (1 << 0) >> /* >> * Indicates that all dependencies of the panel are on: >> * >> @@ -4728,6 +4717,17 @@ enum { >> #define PP_SEQUENCE_STATE_ON_S1_3 (0xb << 0) >> #define PP_SEQUENCE_STATE_RESET (0xf << 0) >> >> +#define _PP_CONTROL_1 0xc7204 >> +#define _PP_CONTROL_2 0xc7304 >> +#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \ >> + _PP_CONTROL_2) >> +#define POWER_CYCLE_DELAY_MASK(0x1f << 4) >> +#define POWER_CYCLE_DELAY_SHIFT 4 >> +#define VDD_OVERRIDE_FORCE(1 << 3) >> +#define BACKLIGHT_ENABLE (1 << 2) >> +#define PWR_DOWN_ON_RESET (1 << 1) >> +#define PWR_STATE_TARGET (1 << 0) > > This entire register looks 100% redundant. Just nuke the whole thing? Needed in the future? BR, Jani. > >> + >> #define _PP_CONTROL 0x61204 >> #define PP_CONTROL(pps_idx) _MMIO_PPS(pps_idx, _PP_CONTROL) >> #define PANEL_UNLOCK_REGS (0xabcd << 16) >> -- >> 2.20.1 -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL
On Fri, Mar 01, 2019 at 05:14:05PM -0800, Lucas De Marchi wrote: > This register was placed in the middle of the PP_STATUS definition. Move > it down together with PP_CONTROL and fix the aligment of the bit > definition (as per documentation it should be 2 spaces instead of 1). > > Signed-off-by: Lucas De Marchi > --- > drivers/gpu/drm/i915/i915_reg.h | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c9b868347481..bbbc0649a180 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4692,17 +4692,6 @@ enum { > #define _PP_STATUS 0x61200 > #define PP_STATUS(pps_idx) _MMIO_PPS(pps_idx, _PP_STATUS) > #define PP_ON (1 << 31) > - > -#define _PP_CONTROL_10xc7204 > -#define _PP_CONTROL_20xc7304 > -#define ICP_PP_CONTROL(x)_MMIO(((x) == 1) ? _PP_CONTROL_1 : \ > - _PP_CONTROL_2) > -#define POWER_CYCLE_DELAY_MASK (0x1f << 4) > -#define POWER_CYCLE_DELAY_SHIFT 4 > -#define VDD_OVERRIDE_FORCE (1 << 3) > -#define BACKLIGHT_ENABLE(1 << 2) > -#define PWR_DOWN_ON_RESET (1 << 1) > -#define PWR_STATE_TARGET(1 << 0) > /* > * Indicates that all dependencies of the panel are on: > * > @@ -4728,6 +4717,17 @@ enum { > #define PP_SEQUENCE_STATE_ON_S1_3 (0xb << 0) > #define PP_SEQUENCE_STATE_RESET(0xf << 0) > > +#define _PP_CONTROL_10xc7204 > +#define _PP_CONTROL_20xc7304 > +#define ICP_PP_CONTROL(x)_MMIO(((x) == 1) ? _PP_CONTROL_1 : \ > + _PP_CONTROL_2) > +#define POWER_CYCLE_DELAY_MASK (0x1f << 4) > +#define POWER_CYCLE_DELAY_SHIFT4 > +#define VDD_OVERRIDE_FORCE (1 << 3) > +#define BACKLIGHT_ENABLE (1 << 2) > +#define PWR_DOWN_ON_RESET (1 << 1) > +#define PWR_STATE_TARGET (1 << 0) This entire register looks 100% redundant. Just nuke the whole thing? > + > #define _PP_CONTROL 0x61204 > #define PP_CONTROL(pps_idx) _MMIO_PPS(pps_idx, _PP_CONTROL) > #define PANEL_UNLOCK_REGS (0xabcd << 16) > -- > 2.20.1 -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: fix placement of ICP_PP_CONTROL
This register was placed in the middle of the PP_STATUS definition. Move it down together with PP_CONTROL and fix the aligment of the bit definition (as per documentation it should be 2 spaces instead of 1). Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/i915/i915_reg.h | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c9b868347481..bbbc0649a180 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4692,17 +4692,6 @@ enum { #define _PP_STATUS 0x61200 #define PP_STATUS(pps_idx) _MMIO_PPS(pps_idx, _PP_STATUS) #define PP_ON(1 << 31) - -#define _PP_CONTROL_1 0xc7204 -#define _PP_CONTROL_2 0xc7304 -#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \ - _PP_CONTROL_2) -#define POWER_CYCLE_DELAY_MASK(0x1f << 4) -#define POWER_CYCLE_DELAY_SHIFT 4 -#define VDD_OVERRIDE_FORCE(1 << 3) -#define BACKLIGHT_ENABLE (1 << 2) -#define PWR_DOWN_ON_RESET (1 << 1) -#define PWR_STATE_TARGET (1 << 0) /* * Indicates that all dependencies of the panel are on: * @@ -4728,6 +4717,17 @@ enum { #define PP_SEQUENCE_STATE_ON_S1_3(0xb << 0) #define PP_SEQUENCE_STATE_RESET (0xf << 0) +#define _PP_CONTROL_1 0xc7204 +#define _PP_CONTROL_2 0xc7304 +#define ICP_PP_CONTROL(x) _MMIO(((x) == 1) ? _PP_CONTROL_1 : \ + _PP_CONTROL_2) +#define POWER_CYCLE_DELAY_MASK (0x1f << 4) +#define POWER_CYCLE_DELAY_SHIFT 4 +#define VDD_OVERRIDE_FORCE (1 << 3) +#define BACKLIGHT_ENABLE (1 << 2) +#define PWR_DOWN_ON_RESET(1 << 1) +#define PWR_STATE_TARGET (1 << 0) + #define _PP_CONTROL0x61204 #define PP_CONTROL(pps_idx)_MMIO_PPS(pps_idx, _PP_CONTROL) #define PANEL_UNLOCK_REGS (0xabcd << 16) -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx