* [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT @ 2015-06-05 13:00 Minu Mathai 2015-06-05 13:08 ` Ville Syrjälä 0 siblings, 1 reply; 12+ messages in thread From: Minu Mathai @ 2015-06-05 13:00 UTC (permalink / raw) To: intel-gfx; +Cc: Minu From: Minu <minu.mathai@intel.com> Display CRCs were not readable because the register defintions for PORT_DFT_I9XX and PORT_DFT2_G4X were wrong. MMIO offset needs to be added to these register offsets to fix them. Issue: GMINL-6869 Signed-off-by: Minu Mathai <minu.mathai@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 7213224..c327c7c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells { #define PCH_HDMIC 0xe1150 #define PCH_HDMID 0xe1160 -#define PORT_DFT_I9XX 0x61150 +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150) #define DC_BALANCE_RESET (1 << 25) #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154) #define DC_BALANCE_RESET_VLV (1 << 31) -- 1.9.1 --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-05 13:00 [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT Minu Mathai @ 2015-06-05 13:08 ` Ville Syrjälä 2015-06-08 13:23 ` Mathai, Minu 2015-06-09 15:01 ` Dave Gordon 0 siblings, 2 replies; 12+ messages in thread From: Ville Syrjälä @ 2015-06-05 13:08 UTC (permalink / raw) To: Minu Mathai; +Cc: intel-gfx On Fri, Jun 05, 2015 at 02:00:24PM +0100, Minu Mathai wrote: > From: Minu <minu.mathai@intel.com> > > Display CRCs were not readable because the register defintions > for PORT_DFT_I9XX and PORT_DFT2_G4X were wrong. > MMIO offset needs to be added to these register offsets to fix them. > > Issue: GMINL-6869 > Signed-off-by: Minu Mathai <minu.mathai@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 7213224..c327c7c 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells { > #define PCH_HDMIC 0xe1150 > #define PCH_HDMID 0xe1160 > > -#define PORT_DFT_I9XX 0x61150 > +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150) PORT_DFT_I9XX isn't used on VLV/CHV, so this doesn't change anything. > #define DC_BALANCE_RESET (1 << 25) > #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154) > #define DC_BALANCE_RESET_VLV (1 << 31) > -- > 1.9.1 > > --------------------------------------------------------------------- > Intel Corporation (UK) Limited > Registered No. 1134945 (England) > Registered Office: Pipers Way, Swindon SN3 1RJ > VAT No: 860 2173 47 > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-05 13:08 ` Ville Syrjälä @ 2015-06-08 13:23 ` Mathai, Minu 2015-06-08 13:48 ` Jani Nikula 2015-06-09 15:01 ` Dave Gordon 1 sibling, 1 reply; 12+ messages in thread From: Mathai, Minu @ 2015-06-08 13:23 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org This change is needed for some hardware composer tests in chv. -----Original Message----- From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] Sent: Friday, June 5, 2015 2:08 PM To: Mathai, Minu Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT On Fri, Jun 05, 2015 at 02:00:24PM +0100, Minu Mathai wrote: > From: Minu <minu.mathai@intel.com> > > Display CRCs were not readable because the register defintions for > PORT_DFT_I9XX and PORT_DFT2_G4X were wrong. > MMIO offset needs to be added to these register offsets to fix them. > > Issue: GMINL-6869 > Signed-off-by: Minu Mathai <minu.mathai@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h index 7213224..c327c7c 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells { > #define PCH_HDMIC 0xe1150 > #define PCH_HDMID 0xe1160 > > -#define PORT_DFT_I9XX 0x61150 > +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150) PORT_DFT_I9XX isn't used on VLV/CHV, so this doesn't change anything. > #define DC_BALANCE_RESET (1 << 25) > #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154) > #define DC_BALANCE_RESET_VLV (1 << 31) > -- > 1.9.1 > > --------------------------------------------------------------------- > Intel Corporation (UK) Limited > Registered No. 1134945 (England) > Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-08 13:23 ` Mathai, Minu @ 2015-06-08 13:48 ` Jani Nikula 2015-06-09 12:06 ` Mathai, Minu 0 siblings, 1 reply; 12+ messages in thread From: Jani Nikula @ 2015-06-08 13:48 UTC (permalink / raw) To: Mathai, Minu, Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org On Mon, 08 Jun 2015, "Mathai, Minu" <minu.mathai@intel.com> wrote: > This change is needed for some hardware composer tests in chv. As Ville said, this #define is not used by the upstream kernel on byt/chv. Maybe you have some out-of-tree patches using that? BR, Jani. > > -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > Sent: Friday, June 5, 2015 2:08 PM > To: Mathai, Minu > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT > > On Fri, Jun 05, 2015 at 02:00:24PM +0100, Minu Mathai wrote: >> From: Minu <minu.mathai@intel.com> >> >> Display CRCs were not readable because the register defintions for >> PORT_DFT_I9XX and PORT_DFT2_G4X were wrong. >> MMIO offset needs to be added to these register offsets to fix them. >> >> Issue: GMINL-6869 >> Signed-off-by: Minu Mathai <minu.mathai@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h index 7213224..c327c7c 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells { >> #define PCH_HDMIC 0xe1150 >> #define PCH_HDMID 0xe1160 >> >> -#define PORT_DFT_I9XX 0x61150 >> +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150) > > PORT_DFT_I9XX isn't used on VLV/CHV, so this doesn't change anything. > > >> #define DC_BALANCE_RESET (1 << 25) >> #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154) >> #define DC_BALANCE_RESET_VLV (1 << 31) >> -- >> 1.9.1 >> >> --------------------------------------------------------------------- >> Intel Corporation (UK) Limited >> Registered No. 1134945 (England) >> Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 >> >> This e-mail and any attachments may contain confidential material for >> the sole use of the intended recipient(s). Any review or distribution >> by others is strictly prohibited. If you are not the intended >> recipient, please contact the sender and delete all copies. >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-08 13:48 ` Jani Nikula @ 2015-06-09 12:06 ` Mathai, Minu 2015-06-09 12:16 ` Ville Syrjälä 0 siblings, 1 reply; 12+ messages in thread From: Mathai, Minu @ 2015-06-09 12:06 UTC (permalink / raw) To: Jani Nikula, Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org The hardware composer test which found the problem with this register definition is used in Android testing and isn't open source. However fixing this register definition will stop potential problems for future use cases and would reduce the rebase effort for our Android tree. -----Original Message----- From: Jani Nikula [mailto:jani.nikula@linux.intel.com] Sent: Monday, June 8, 2015 2:48 PM To: Mathai, Minu; Ville Syrjälä Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT On Mon, 08 Jun 2015, "Mathai, Minu" <minu.mathai@intel.com> wrote: > This change is needed for some hardware composer tests in chv. As Ville said, this #define is not used by the upstream kernel on byt/chv. Maybe you have some out-of-tree patches using that? BR, Jani. > > -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > Sent: Friday, June 5, 2015 2:08 PM > To: Mathai, Minu > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correcting the reg > definitions for PORT_DFT > > On Fri, Jun 05, 2015 at 02:00:24PM +0100, Minu Mathai wrote: >> From: Minu <minu.mathai@intel.com> >> >> Display CRCs were not readable because the register defintions for >> PORT_DFT_I9XX and PORT_DFT2_G4X were wrong. >> MMIO offset needs to be added to these register offsets to fix them. >> >> Issue: GMINL-6869 >> Signed-off-by: Minu Mathai <minu.mathai@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h index 7213224..c327c7c 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells { >> #define PCH_HDMIC 0xe1150 >> #define PCH_HDMID 0xe1160 >> >> -#define PORT_DFT_I9XX 0x61150 >> +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150) > > PORT_DFT_I9XX isn't used on VLV/CHV, so this doesn't change anything. > > >> #define DC_BALANCE_RESET (1 << 25) >> #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154) >> #define DC_BALANCE_RESET_VLV (1 << 31) >> -- >> 1.9.1 >> >> --------------------------------------------------------------------- >> Intel Corporation (UK) Limited >> Registered No. 1134945 (England) >> Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 >> >> This e-mail and any attachments may contain confidential material for >> the sole use of the intended recipient(s). Any review or distribution >> by others is strictly prohibited. If you are not the intended >> recipient, please contact the sender and delete all copies. >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-09 12:06 ` Mathai, Minu @ 2015-06-09 12:16 ` Ville Syrjälä 2015-06-09 13:32 ` Mathai, Minu 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2015-06-09 12:16 UTC (permalink / raw) To: Mathai, Minu; +Cc: intel-gfx@lists.freedesktop.org On Tue, Jun 09, 2015 at 12:06:36PM +0000, Mathai, Minu wrote: > The hardware composer test which found the problem with this register definition is used in Android testing and isn't open source. > However fixing this register definition will stop potential problems for future use cases and would reduce the rebase effort for our Android tree. What are you testing with this register? The register isn't even listed in any VLV/CHV documentation so I have no idea what you would even do with it. > > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > Sent: Monday, June 8, 2015 2:48 PM > To: Mathai, Minu; Ville Syrjälä > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT > > On Mon, 08 Jun 2015, "Mathai, Minu" <minu.mathai@intel.com> wrote: > > This change is needed for some hardware composer tests in chv. > > As Ville said, this #define is not used by the upstream kernel on byt/chv. Maybe you have some out-of-tree patches using that? > > BR, > Jani. > > > > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > > Sent: Friday, June 5, 2015 2:08 PM > > To: Mathai, Minu > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correcting the reg > > definitions for PORT_DFT > > > > On Fri, Jun 05, 2015 at 02:00:24PM +0100, Minu Mathai wrote: > >> From: Minu <minu.mathai@intel.com> > >> > >> Display CRCs were not readable because the register defintions for > >> PORT_DFT_I9XX and PORT_DFT2_G4X were wrong. > >> MMIO offset needs to be added to these register offsets to fix them. > >> > >> Issue: GMINL-6869 > >> Signed-off-by: Minu Mathai <minu.mathai@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_reg.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h > >> b/drivers/gpu/drm/i915/i915_reg.h index 7213224..c327c7c 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells { > >> #define PCH_HDMIC 0xe1150 > >> #define PCH_HDMID 0xe1160 > >> > >> -#define PORT_DFT_I9XX 0x61150 > >> +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150) > > > > PORT_DFT_I9XX isn't used on VLV/CHV, so this doesn't change anything. > > > > > >> #define DC_BALANCE_RESET (1 << 25) > >> #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154) > >> #define DC_BALANCE_RESET_VLV (1 << 31) > >> -- > >> 1.9.1 > >> > >> --------------------------------------------------------------------- > >> Intel Corporation (UK) Limited > >> Registered No. 1134945 (England) > >> Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 > >> > >> This e-mail and any attachments may contain confidential material for > >> the sole use of the intended recipient(s). Any review or distribution > >> by others is strictly prohibited. If you are not the intended > >> recipient, please contact the sender and delete all copies. > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-09 12:16 ` Ville Syrjälä @ 2015-06-09 13:32 ` Mathai, Minu 0 siblings, 0 replies; 12+ messages in thread From: Mathai, Minu @ 2015-06-09 13:32 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org Agreed .We don't need this Thanks Minu -----Original Message----- From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] Sent: Tuesday, June 9, 2015 1:17 PM To: Mathai, Minu Cc: Jani Nikula; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT On Tue, Jun 09, 2015 at 12:06:36PM +0000, Mathai, Minu wrote: > The hardware composer test which found the problem with this register definition is used in Android testing and isn't open source. > However fixing this register definition will stop potential problems for future use cases and would reduce the rebase effort for our Android tree. What are you testing with this register? The register isn't even listed in any VLV/CHV documentation so I have no idea what you would even do with it. > > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > Sent: Monday, June 8, 2015 2:48 PM > To: Mathai, Minu; Ville Syrjälä > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correcting the reg > definitions for PORT_DFT > > On Mon, 08 Jun 2015, "Mathai, Minu" <minu.mathai@intel.com> wrote: > > This change is needed for some hardware composer tests in chv. > > As Ville said, this #define is not used by the upstream kernel on byt/chv. Maybe you have some out-of-tree patches using that? > > BR, > Jani. > > > > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > > Sent: Friday, June 5, 2015 2:08 PM > > To: Mathai, Minu > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correcting the reg > > definitions for PORT_DFT > > > > On Fri, Jun 05, 2015 at 02:00:24PM +0100, Minu Mathai wrote: > >> From: Minu <minu.mathai@intel.com> > >> > >> Display CRCs were not readable because the register defintions for > >> PORT_DFT_I9XX and PORT_DFT2_G4X were wrong. > >> MMIO offset needs to be added to these register offsets to fix them. > >> > >> Issue: GMINL-6869 > >> Signed-off-by: Minu Mathai <minu.mathai@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_reg.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h > >> b/drivers/gpu/drm/i915/i915_reg.h index 7213224..c327c7c 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells { > >> #define PCH_HDMIC 0xe1150 > >> #define PCH_HDMID 0xe1160 > >> > >> -#define PORT_DFT_I9XX 0x61150 > >> +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150) > > > > PORT_DFT_I9XX isn't used on VLV/CHV, so this doesn't change anything. > > > > > >> #define DC_BALANCE_RESET (1 << 25) > >> #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154) > >> #define DC_BALANCE_RESET_VLV (1 << 31) > >> -- > >> 1.9.1 > >> > >> ------------------------------------------------------------------- > >> -- > >> Intel Corporation (UK) Limited > >> Registered No. 1134945 (England) > >> Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 > >> > >> This e-mail and any attachments may contain confidential material > >> for the sole use of the intended recipient(s). Any review or > >> distribution by others is strictly prohibited. If you are not the > >> intended recipient, please contact the sender and delete all copies. > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-05 13:08 ` Ville Syrjälä 2015-06-08 13:23 ` Mathai, Minu @ 2015-06-09 15:01 ` Dave Gordon 2015-06-09 15:24 ` Ville Syrjälä 2015-06-10 8:09 ` Jani Nikula 1 sibling, 2 replies; 12+ messages in thread From: Dave Gordon @ 2015-06-09 15:01 UTC (permalink / raw) To: Ville Syrjälä, Minu Mathai; +Cc: intel-gfx On 05/06/15 14:08, Ville Syrjälä wrote: > On Fri, Jun 05, 2015 at 02:00:24PM +0100, Minu Mathai wrote: >> From: Minu <minu.mathai@intel.com> >> >> Display CRCs were not readable because the register defintions >> for PORT_DFT_I9XX and PORT_DFT2_G4X were wrong. >> MMIO offset needs to be added to these register offsets to fix them. >> >> Issue: GMINL-6869 >> Signed-off-by: Minu Mathai <minu.mathai@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index 7213224..c327c7c 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells { >> #define PCH_HDMIC 0xe1150 >> #define PCH_HDMID 0xe1160 >> >> -#define PORT_DFT_I9XX 0x61150 >> +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150) > > PORT_DFT_I9XX isn't used on VLV/CHV, so this doesn't change anything. > >> #define DC_BALANCE_RESET (1 << 25) >> #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154) >> #define DC_BALANCE_RESET_VLV (1 << 31) Regardless of whether it's used, we have an inconsistency between the definitions of PORT_DFT_I9XX and PORT_DFT2_G4X -- one includes the mmio_offset and the other doesn't. Personally I think the #define with an implicit dependency on an object called "dev_priv" is really ugly and we should move away from that style rather than adding mode of them, but that's a lot of work. As Minu says PORT_DFT_I9XX isn't really needed after all, can we just delete it to remove the inconsistency? .Dave. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-09 15:01 ` Dave Gordon @ 2015-06-09 15:24 ` Ville Syrjälä 2015-06-10 8:09 ` Jani Nikula 1 sibling, 0 replies; 12+ messages in thread From: Ville Syrjälä @ 2015-06-09 15:24 UTC (permalink / raw) To: Dave Gordon; +Cc: Minu Mathai, intel-gfx On Tue, Jun 09, 2015 at 04:01:29PM +0100, Dave Gordon wrote: > On 05/06/15 14:08, Ville Syrjälä wrote: > > On Fri, Jun 05, 2015 at 02:00:24PM +0100, Minu Mathai wrote: > >> From: Minu <minu.mathai@intel.com> > >> > >> Display CRCs were not readable because the register defintions > >> for PORT_DFT_I9XX and PORT_DFT2_G4X were wrong. > >> MMIO offset needs to be added to these register offsets to fix them. > >> > >> Issue: GMINL-6869 > >> Signed-off-by: Minu Mathai <minu.mathai@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_reg.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > >> index 7213224..c327c7c 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells { > >> #define PCH_HDMIC 0xe1150 > >> #define PCH_HDMID 0xe1160 > >> > >> -#define PORT_DFT_I9XX 0x61150 > >> +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150) > > > > PORT_DFT_I9XX isn't used on VLV/CHV, so this doesn't change anything. > > > >> #define DC_BALANCE_RESET (1 << 25) > >> #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154) > >> #define DC_BALANCE_RESET_VLV (1 << 31) > > Regardless of whether it's used, we have an inconsistency between the > definitions of PORT_DFT_I9XX and PORT_DFT2_G4X -- one includes the > mmio_offset and the other doesn't. Personally I think the #define with > an implicit dependency on an object called "dev_priv" is really ugly and > we should move away from that style rather than adding mode of them, but > that's a lot of work. > > As Minu says PORT_DFT_I9XX isn't really needed after all, can we just > delete it to remove the inconsistency? No, it's used on g4x. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-09 15:01 ` Dave Gordon 2015-06-09 15:24 ` Ville Syrjälä @ 2015-06-10 8:09 ` Jani Nikula 2015-06-10 12:27 ` Dave Gordon 1 sibling, 1 reply; 12+ messages in thread From: Jani Nikula @ 2015-06-10 8:09 UTC (permalink / raw) To: Dave Gordon, Ville Syrjälä, Minu Mathai; +Cc: intel-gfx On Tue, 09 Jun 2015, Dave Gordon <david.s.gordon@intel.com> wrote: > Regardless of whether it's used, we have an inconsistency between the > definitions of PORT_DFT_I9XX and PORT_DFT2_G4X -- one includes the > mmio_offset and the other doesn't. It's not inconsistent, it's consistent on another level: We've settled on including the mmio_offset only for macros that need it. If a macro is relevant only on platforms that all have the same mmio offset, the offset is included statically (currently 0 or VLV_DISPLAY_BASE). dev_priv->info.display_mmio_offset is only used when the macro is relevant on platforms with different mmio offsets. This we try to follow consistently. > Personally I think the #define with an implicit dependency on an > object called "dev_priv" is really ugly and we should move away from > that style rather than adding mode of them, but that's a lot of work. Agreed in principle, but I think we've lost the battle. It's way too much churn, and makes a lot of code really ugly. Compare these: I915_WRITE(FOOBAR, I915_READ(FOOBAR) | FOOBAR_ENABLE); I915_WRITE(dev_priv, FOOBAR(dev_priv), I915_READ(dev_priv, FOOBAR(dev_priv)) | FOOBAR_ENABLE); We'll try to focus on only requiring "dev_priv" implicitly and nothing else, and where a parameter does get passed, we'll try to make it "dev_priv" as that pretty much has to be around anyway. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-10 8:09 ` Jani Nikula @ 2015-06-10 12:27 ` Dave Gordon 2015-06-10 12:50 ` Jani Nikula 0 siblings, 1 reply; 12+ messages in thread From: Dave Gordon @ 2015-06-10 12:27 UTC (permalink / raw) To: Jani Nikula, Ville Syrjälä, Minu Mathai; +Cc: intel-gfx On 10/06/15 09:09, Jani Nikula wrote: > On Tue, 09 Jun 2015, Dave Gordon <david.s.gordon@intel.com> wrote: >> Regardless of whether it's used, we have an inconsistency between the >> definitions of PORT_DFT_I9XX and PORT_DFT2_G4X -- one includes the >> mmio_offset and the other doesn't. > > It's not inconsistent, it's consistent on another level: > > We've settled on including the mmio_offset only for macros that need > it. If a macro is relevant only on platforms that all have the same mmio > offset, the offset is included statically (currently 0 or > VLV_DISPLAY_BASE). dev_priv->info.display_mmio_offset is only used when > the macro is relevant on platforms with different mmio offsets. This we > try to follow consistently. So you only have a problem when a previously-statically located block turns into something that can be (and is) moved around on different platforms ... I suppose that shouldn't happen too often. >> Personally I think the #define with an implicit dependency on an >> object called "dev_priv" is really ugly and we should move away from >> that style rather than adding more of them, but that's a lot of work. > > Agreed in principle, but I think we've lost the battle. It's way too > much churn, and makes a lot of code really ugly. Compare these: > > I915_WRITE(FOOBAR, I915_READ(FOOBAR) | FOOBAR_ENABLE); > > I915_WRITE(dev_priv, FOOBAR(dev_priv), > I915_READ(dev_priv, FOOBAR(dev_priv)) | FOOBAR_ENABLE); I'd rather write uint32_t foobar = I915_DISP_REG_READ(dev_priv, FOOBAR); I915_DISP_REG_WRITE(dev_priv, foobar | FOOBAR_ENABLE); And if there were very many places where that read-modify-write was used, we could define that as #define I915_DISP_REG_BITSET(dev_priv, reg, bits) ... Of course it would require people to use the right macro according to whether the register was part of the display block, the GT block, or however many other areas there are that might be independently relocated ... but it might be more future proof :) > We'll try to focus on only requiring "dev_priv" implicitly and nothing > else, and where a parameter does get passed, we'll try to make it > "dev_priv" as that pretty much has to be around anyway. > > BR, > Jani. Well I guess we're stuck with 'dev_priv' for the foreseeable future :( Hmmm ... we have quite a bit of code that passes 'dev' around rather than dev_priv, where the (usually static) called function immediately uses it to derive dev_priv, and where the only other uses in the called function are in calls to INTEL_INFO(dev) and its derivatives. For example: static int get_context_size(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; int ret; u32 reg; switch (INTEL_INFO(dev)->gen) { case 6: reg = I915_READ(CXT_SIZE); ret = GEN6_CXT_TOTAL_SIZE(reg) * 64; break; case 7: reg = I915_READ(GEN7_CXT_SIZE); if (IS_HASWELL(dev)) ret = HSW_CXT_TOTAL_SIZE; else ret = GEN7_CXT_TOTAL_SIZE(reg) * 64; break; case 8: ret = GEN8_CXT_TOTAL_SIZE; break; default: BUG(); } return ret; } But since INTEL_INFO() can take 'dev_priv' as a parameter (as an alternative to 'dev'), all the uses of 'dev' here could be replaced by 'dev_priv' and then the parameter changed to pass that directly, thus potentially eliminating quite a few extra memory references. Applied driver-wide, that micro-optimisation might add up to something useful :) .Dave. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT 2015-06-10 12:27 ` Dave Gordon @ 2015-06-10 12:50 ` Jani Nikula 0 siblings, 0 replies; 12+ messages in thread From: Jani Nikula @ 2015-06-10 12:50 UTC (permalink / raw) To: Dave Gordon, Ville Syrjälä, Minu Mathai; +Cc: intel-gfx On Wed, 10 Jun 2015, Dave Gordon <david.s.gordon@intel.com> wrote: > But since INTEL_INFO() can take 'dev_priv' as a parameter (as an > alternative to 'dev'), all the uses of 'dev' here could be replaced by > 'dev_priv' and then the parameter changed to pass that directly, thus > potentially eliminating quite a few extra memory references. Applied > driver-wide, that micro-optimisation might add up to something useful :) I think we haven't done that driver-wide because it would be more disruptive than beneficial, but we're slowly moving at that direction whenever we change stuff. And this is exactly the reason why Chris hacked INTEL_INFO et al. to accept either dev or dev_priv. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-06-10 12:53 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-05 13:00 [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT Minu Mathai 2015-06-05 13:08 ` Ville Syrjälä 2015-06-08 13:23 ` Mathai, Minu 2015-06-08 13:48 ` Jani Nikula 2015-06-09 12:06 ` Mathai, Minu 2015-06-09 12:16 ` Ville Syrjälä 2015-06-09 13:32 ` Mathai, Minu 2015-06-09 15:01 ` Dave Gordon 2015-06-09 15:24 ` Ville Syrjälä 2015-06-10 8:09 ` Jani Nikula 2015-06-10 12:27 ` Dave Gordon 2015-06-10 12:50 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox