Re: [Intel-gfx] [PATCH 5/5] drm/i915: Make use of unused variable in hsw_write_infoframe()

[Jani Nikula]

On Wed, 14 Feb 2018, Ville Syrjälä  wrote:
> On Tue, Feb 13, 2018 at 04:29:17PM -0800, José Roberto de Souza wrote:
>> data_reg was not being used but it can be used to reduce the number of
>> calls to hsw_dip_data_reg() and just increment the reg by the size of
>> uint32.
>> 
>> Signed-off-by: José Roberto de Souza 
>> ---
>>  drivers/gpu/drm/i915/intel_hdmi.c | 12 
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
>> b/drivers/gpu/drm/i915/intel_hdmi.c
>> index f5d7bfb43006..ef258eac8ae8 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -393,15 +393,11 @@ static void hsw_write_infoframe(struct drm_encoder 
>> *encoder,
>>  I915_WRITE(ctl_reg, val);
>>  
>>  mmiowb();
>> -for (i = 0; i < len; i += 4) {
>> -I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
>> -type, i >> 2), *data);a
>
> i>>2 != 0
>
> just kill 'data_reg'

What Ville says.

Furthermore, you're *not* supposed to mess with the contents of
i915_reg_t. That we use a typedef to begin with means "don't touch the
guts of this one".

BR,
Jani.

>
>> -data++;
>> -}
>> +for (i = 0; i < len; i += 4, data++, data_reg.reg += 4)
>> +I915_WRITE(data_reg, *data);
>>  /* Write every possible data byte to force correct ECC calculation. */
>> -for (; i < data_size; i += 4)
>> -I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
>> -type, i >> 2), 0);
>> +for (; i < data_size; i += 4, data_reg.reg += 4)
>> +I915_WRITE(data_reg, 0);
>>  mmiowb();
>>  
>>  val |= hsw_infoframe_enable(type);
>> -- 
>> 2.16.1
>> 
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915: Make use of unused variable in hsw_write_infoframe()

[Ville Syrjälä]

On Wed, Feb 14, 2018 at 01:37:22PM +, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-02-14 13:33:03)
> > On Tue, Feb 13, 2018 at 04:29:17PM -0800, José Roberto de Souza wrote:
> > > data_reg was not being used but it can be used to reduce the number of
> > > calls to hsw_dip_data_reg() and just increment the reg by the size of
> > > uint32.
> > > 
> > > Signed-off-by: José Roberto de Souza 
> > > ---
> > >  drivers/gpu/drm/i915/intel_hdmi.c | 12 
> > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> > > b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index f5d7bfb43006..ef258eac8ae8 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -393,15 +393,11 @@ static void hsw_write_infoframe(struct drm_encoder 
> > > *encoder,
> > >   I915_WRITE(ctl_reg, val);
> > >  
> > >   mmiowb();
> 
> mmiowb() is just a compiler barrier on x86, and meaningless wrt to our
> uncached mmio. If you need a full mmio barrier, use POSTING_READ().

Yeah, someone should nuke all the mmiowb()s from the infoframe code.
I still don't understand what they were supposed to achieve.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915: Make use of unused variable in hsw_write_infoframe()

[Chris Wilson]

Quoting Ville Syrjälä (2018-02-14 13:33:03)
> On Tue, Feb 13, 2018 at 04:29:17PM -0800, José Roberto de Souza wrote:
> > data_reg was not being used but it can be used to reduce the number of
> > calls to hsw_dip_data_reg() and just increment the reg by the size of
> > uint32.
> > 
> > Signed-off-by: José Roberto de Souza 
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 12 
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index f5d7bfb43006..ef258eac8ae8 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -393,15 +393,11 @@ static void hsw_write_infoframe(struct drm_encoder 
> > *encoder,
> >   I915_WRITE(ctl_reg, val);
> >  
> >   mmiowb();

mmiowb() is just a compiler barrier on x86, and meaningless wrt to our
uncached mmio. If you need a full mmio barrier, use POSTING_READ().
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915: Make use of unused variable in hsw_write_infoframe()

[Ville Syrjälä]

On Tue, Feb 13, 2018 at 04:29:17PM -0800, José Roberto de Souza wrote:
> data_reg was not being used but it can be used to reduce the number of
> calls to hsw_dip_data_reg() and just increment the reg by the size of
> uint32.
> 
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index f5d7bfb43006..ef258eac8ae8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -393,15 +393,11 @@ static void hsw_write_infoframe(struct drm_encoder 
> *encoder,
>   I915_WRITE(ctl_reg, val);
>  
>   mmiowb();
> - for (i = 0; i < len; i += 4) {
> - I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
> - type, i >> 2), *data);a

i>>2 != 0

just kill 'data_reg'

> - data++;
> - }
> + for (i = 0; i < len; i += 4, data++, data_reg.reg += 4)
> + I915_WRITE(data_reg, *data);
>   /* Write every possible data byte to force correct ECC calculation. */
> - for (; i < data_size; i += 4)
> - I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
> - type, i >> 2), 0);
> + for (; i < data_size; i += 4, data_reg.reg += 4)
> + I915_WRITE(data_reg, 0);
>   mmiowb();
>  
>   val |= hsw_infoframe_enable(type);
> -- 
> 2.16.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 5/5] drm/i915: Make use of unused variable in hsw_write_infoframe()

[José Roberto de Souza]

data_reg was not being used but it can be used to reduce the number of
calls to hsw_dip_data_reg() and just increment the reg by the size of
uint32.

Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index f5d7bfb43006..ef258eac8ae8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -393,15 +393,11 @@ static void hsw_write_infoframe(struct drm_encoder 
*encoder,
I915_WRITE(ctl_reg, val);
 
mmiowb();
-   for (i = 0; i < len; i += 4) {
-   I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
-   type, i >> 2), *data);
-   data++;
-   }
+   for (i = 0; i < len; i += 4, data++, data_reg.reg += 4)
+   I915_WRITE(data_reg, *data);
/* Write every possible data byte to force correct ECC calculation. */
-   for (; i < data_size; i += 4)
-   I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
-   type, i >> 2), 0);
+   for (; i < data_size; i += 4, data_reg.reg += 4)
+   I915_WRITE(data_reg, 0);
mmiowb();
 
val |= hsw_infoframe_enable(type);
-- 
2.16.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx