Re: [Intel-gfx] [PATCH v2 6/7] drm/i915: No 10bit gamma on desktop gen3 parts

2023-05-26 Thread Ville Syrjälä
On Thu, May 25, 2023 at 09:25:46PM +, Shankar, Uma wrote:
> 
> 
> > -Original Message-
> > From: Intel-gfx  On Behalf Of 
> > Ville Syrjala
> > Sent: Thursday, April 13, 2023 10:19 PM
> > To: intel-...@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org
> > Subject: [Intel-gfx] [PATCH v2 6/7] drm/i915: No 10bit gamma on desktop 
> > gen3 parts
> > 
> > From: Ville Syrjälä 
> > 
> > Apparently desktop gen3 parts don't support the 10bit gamma mode at all. 
> > Stop
> > claiming otherwise.
> > 
> > As is the case with pipe A on gen3 mobile parts, the PIPECONF gamma mode 
> > bit can
> > be set but it has no effect on the output.
> > 
> > PNV seems to be the only slight exception, but generally the desktop PNV 
> > variant
> > looks more like a mobile part so this is not entirely surprising.
> 
> Couldn't check the relevant spec to re-confirm, trusting your judgment here.

The spec isn't helpful here actually. Real world testing on every
gen3 platform was needed to be sure how this works.

> Reviewed-by: Uma Shankar 

Thanks.

> 
> > Fixes: 67630bacae23 ("drm/i915: Add 10bit gamma mode for gen2/3")
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_pci.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c 
> > b/drivers/gpu/drm/i915/i915_pci.c index
> > cddb6e197972..305c05c3f93b 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -250,13 +250,13 @@ static const struct intel_device_info i865g_info = {
> > .dma_mask_size = 32, \
> > I9XX_PIPE_OFFSETS, \
> > I9XX_CURSOR_OFFSETS, \
> > -   I9XX_COLORS, \
> > GEN_DEFAULT_PAGE_SIZES, \
> > GEN_DEFAULT_REGIONS
> > 
> >  static const struct intel_device_info i915g_info = {
> > GEN3_FEATURES,
> > PLATFORM(INTEL_I915G),
> > +   I845_COLORS,
> > .has_coherent_ggtt = false,
> > .display.cursor_needs_physical = 1,
> > .display.has_overlay = 1,
> > @@ -268,6 +268,7 @@ static const struct intel_device_info i915g_info = {  
> > static
> > const struct intel_device_info i915gm_info = {
> > GEN3_FEATURES,
> > PLATFORM(INTEL_I915GM),
> > +   I9XX_COLORS,
> > .is_mobile = 1,
> > .display.cursor_needs_physical = 1,
> > .display.has_overlay = 1,
> > @@ -281,6 +282,7 @@ static const struct intel_device_info i915gm_info = {  
> > static
> > const struct intel_device_info i945g_info = {
> > GEN3_FEATURES,
> > PLATFORM(INTEL_I945G),
> > +   I845_COLORS,
> > .display.has_hotplug = 1,
> > .display.cursor_needs_physical = 1,
> > .display.has_overlay = 1,
> > @@ -292,6 +294,7 @@ static const struct intel_device_info i945g_info = {  
> > static
> > const struct intel_device_info i945gm_info = {
> > GEN3_FEATURES,
> > PLATFORM(INTEL_I945GM),
> > +   I9XX_COLORS,
> > .is_mobile = 1,
> > .display.has_hotplug = 1,
> > .display.cursor_needs_physical = 1,
> > @@ -306,6 +309,7 @@ static const struct intel_device_info i945gm_info = {  
> > static
> > const struct intel_device_info g33_info = {
> > GEN3_FEATURES,
> > PLATFORM(INTEL_G33),
> > +   I845_COLORS,
> > .display.has_hotplug = 1,
> > .display.has_overlay = 1,
> > .dma_mask_size = 36,
> > @@ -314,6 +318,7 @@ static const struct intel_device_info g33_info = {  
> > static
> > const struct intel_device_info pnv_g_info = {
> > GEN3_FEATURES,
> > PLATFORM(INTEL_PINEVIEW),
> > +   I9XX_COLORS,
> > .display.has_hotplug = 1,
> > .display.has_overlay = 1,
> > .dma_mask_size = 36,
> > @@ -322,6 +327,7 @@ static const struct intel_device_info pnv_g_info = {  
> > static
> > const struct intel_device_info pnv_m_info = {
> > GEN3_FEATURES,
> > PLATFORM(INTEL_PINEVIEW),
> > +   I9XX_COLORS,
> > .is_mobile = 1,
> > .display.has_hotplug = 1,
> > .display.has_overlay = 1,
> > --
> > 2.39.2
> 

-- 
Ville Syrjälä
Intel


Re: [Intel-gfx] [PATCH v2 6/7] drm/i915: No 10bit gamma on desktop gen3 parts

2023-05-26 Thread Jani Nikula
On Thu, 25 May 2023, "Shankar, Uma"  wrote:
>> -Original Message-
>> From: Intel-gfx  On Behalf Of Ville 
>> Syrjala
>> Sent: Thursday, April 13, 2023 10:19 PM
>> To: intel-...@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>> Subject: [Intel-gfx] [PATCH v2 6/7] drm/i915: No 10bit gamma on desktop gen3 
>> parts
>> 
>> From: Ville Syrjälä 
>> 
>> Apparently desktop gen3 parts don't support the 10bit gamma mode at all. Stop
>> claiming otherwise.
>> 
>> As is the case with pipe A on gen3 mobile parts, the PIPECONF gamma mode bit 
>> can
>> be set but it has no effect on the output.
>> 
>> PNV seems to be the only slight exception, but generally the desktop PNV 
>> variant
>> looks more like a mobile part so this is not entirely surprising.
>
> Couldn't check the relevant spec to re-confirm, trusting your judgment here.
> Reviewed-by: Uma Shankar 

This needs a rebase on display/intel_display_device.[ch] stuff.

BR,
Jani.


>
>> Fixes: 67630bacae23 ("drm/i915: Add 10bit gamma mode for gen2/3")
>> Signed-off-by: Ville Syrjälä 
>> ---
>>  drivers/gpu/drm/i915/i915_pci.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c 
>> b/drivers/gpu/drm/i915/i915_pci.c index
>> cddb6e197972..305c05c3f93b 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -250,13 +250,13 @@ static const struct intel_device_info i865g_info = {
>>  .dma_mask_size = 32, \
>>  I9XX_PIPE_OFFSETS, \
>>  I9XX_CURSOR_OFFSETS, \
>> -I9XX_COLORS, \
>>  GEN_DEFAULT_PAGE_SIZES, \
>>  GEN_DEFAULT_REGIONS
>> 
>>  static const struct intel_device_info i915g_info = {
>>  GEN3_FEATURES,
>>  PLATFORM(INTEL_I915G),
>> +I845_COLORS,
>>  .has_coherent_ggtt = false,
>>  .display.cursor_needs_physical = 1,
>>  .display.has_overlay = 1,
>> @@ -268,6 +268,7 @@ static const struct intel_device_info i915g_info = {  
>> static
>> const struct intel_device_info i915gm_info = {
>>  GEN3_FEATURES,
>>  PLATFORM(INTEL_I915GM),
>> +I9XX_COLORS,
>>  .is_mobile = 1,
>>  .display.cursor_needs_physical = 1,
>>  .display.has_overlay = 1,
>> @@ -281,6 +282,7 @@ static const struct intel_device_info i915gm_info = {  
>> static
>> const struct intel_device_info i945g_info = {
>>  GEN3_FEATURES,
>>  PLATFORM(INTEL_I945G),
>> +I845_COLORS,
>>  .display.has_hotplug = 1,
>>  .display.cursor_needs_physical = 1,
>>  .display.has_overlay = 1,
>> @@ -292,6 +294,7 @@ static const struct intel_device_info i945g_info = {  
>> static
>> const struct intel_device_info i945gm_info = {
>>  GEN3_FEATURES,
>>  PLATFORM(INTEL_I945GM),
>> +I9XX_COLORS,
>>  .is_mobile = 1,
>>  .display.has_hotplug = 1,
>>  .display.cursor_needs_physical = 1,
>> @@ -306,6 +309,7 @@ static const struct intel_device_info i945gm_info = {  
>> static
>> const struct intel_device_info g33_info = {
>>  GEN3_FEATURES,
>>  PLATFORM(INTEL_G33),
>> +I845_COLORS,
>>  .display.has_hotplug = 1,
>>  .display.has_overlay = 1,
>>  .dma_mask_size = 36,
>> @@ -314,6 +318,7 @@ static const struct intel_device_info g33_info = {  
>> static
>> const struct intel_device_info pnv_g_info = {
>>  GEN3_FEATURES,
>>  PLATFORM(INTEL_PINEVIEW),
>> +I9XX_COLORS,
>>  .display.has_hotplug = 1,
>>  .display.has_overlay = 1,
>>  .dma_mask_size = 36,
>> @@ -322,6 +327,7 @@ static const struct intel_device_info pnv_g_info = {  
>> static
>> const struct intel_device_info pnv_m_info = {
>>  GEN3_FEATURES,
>>  PLATFORM(INTEL_PINEVIEW),
>> +I9XX_COLORS,
>>  .is_mobile = 1,
>>  .display.has_hotplug = 1,
>>  .display.has_overlay = 1,
>> --
>> 2.39.2
>

-- 
Jani Nikula, Intel Open Source Graphics Center


RE: [Intel-gfx] [PATCH v2 6/7] drm/i915: No 10bit gamma on desktop gen3 parts

2023-05-25 Thread Shankar, Uma


> -Original Message-
> From: Intel-gfx  On Behalf Of Ville 
> Syrjala
> Sent: Thursday, April 13, 2023 10:19 PM
> To: intel-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 6/7] drm/i915: No 10bit gamma on desktop gen3 
> parts
> 
> From: Ville Syrjälä 
> 
> Apparently desktop gen3 parts don't support the 10bit gamma mode at all. Stop
> claiming otherwise.
> 
> As is the case with pipe A on gen3 mobile parts, the PIPECONF gamma mode bit 
> can
> be set but it has no effect on the output.
> 
> PNV seems to be the only slight exception, but generally the desktop PNV 
> variant
> looks more like a mobile part so this is not entirely surprising.

Couldn't check the relevant spec to re-confirm, trusting your judgment here.
Reviewed-by: Uma Shankar 

> Fixes: 67630bacae23 ("drm/i915: Add 10bit gamma mode for gen2/3")
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_pci.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c 
> b/drivers/gpu/drm/i915/i915_pci.c index
> cddb6e197972..305c05c3f93b 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -250,13 +250,13 @@ static const struct intel_device_info i865g_info = {
>   .dma_mask_size = 32, \
>   I9XX_PIPE_OFFSETS, \
>   I9XX_CURSOR_OFFSETS, \
> - I9XX_COLORS, \
>   GEN_DEFAULT_PAGE_SIZES, \
>   GEN_DEFAULT_REGIONS
> 
>  static const struct intel_device_info i915g_info = {
>   GEN3_FEATURES,
>   PLATFORM(INTEL_I915G),
> + I845_COLORS,
>   .has_coherent_ggtt = false,
>   .display.cursor_needs_physical = 1,
>   .display.has_overlay = 1,
> @@ -268,6 +268,7 @@ static const struct intel_device_info i915g_info = {  
> static
> const struct intel_device_info i915gm_info = {
>   GEN3_FEATURES,
>   PLATFORM(INTEL_I915GM),
> + I9XX_COLORS,
>   .is_mobile = 1,
>   .display.cursor_needs_physical = 1,
>   .display.has_overlay = 1,
> @@ -281,6 +282,7 @@ static const struct intel_device_info i915gm_info = {  
> static
> const struct intel_device_info i945g_info = {
>   GEN3_FEATURES,
>   PLATFORM(INTEL_I945G),
> + I845_COLORS,
>   .display.has_hotplug = 1,
>   .display.cursor_needs_physical = 1,
>   .display.has_overlay = 1,
> @@ -292,6 +294,7 @@ static const struct intel_device_info i945g_info = {  
> static
> const struct intel_device_info i945gm_info = {
>   GEN3_FEATURES,
>   PLATFORM(INTEL_I945GM),
> + I9XX_COLORS,
>   .is_mobile = 1,
>   .display.has_hotplug = 1,
>   .display.cursor_needs_physical = 1,
> @@ -306,6 +309,7 @@ static const struct intel_device_info i945gm_info = {  
> static
> const struct intel_device_info g33_info = {
>   GEN3_FEATURES,
>   PLATFORM(INTEL_G33),
> + I845_COLORS,
>   .display.has_hotplug = 1,
>   .display.has_overlay = 1,
>   .dma_mask_size = 36,
> @@ -314,6 +318,7 @@ static const struct intel_device_info g33_info = {  static
> const struct intel_device_info pnv_g_info = {
>   GEN3_FEATURES,
>   PLATFORM(INTEL_PINEVIEW),
> + I9XX_COLORS,
>   .display.has_hotplug = 1,
>   .display.has_overlay = 1,
>   .dma_mask_size = 36,
> @@ -322,6 +327,7 @@ static const struct intel_device_info pnv_g_info = {  
> static
> const struct intel_device_info pnv_m_info = {
>   GEN3_FEATURES,
>   PLATFORM(INTEL_PINEVIEW),
> + I9XX_COLORS,
>   .is_mobile = 1,
>   .display.has_hotplug = 1,
>   .display.has_overlay = 1,
> --
> 2.39.2