public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] Enable dithering on intel VCH DVO chips on 18bpp panels
@ 2015-03-28 10:07 Thomas Richter
  2015-03-30  9:37 ` Daniel Vetter
  2015-03-30  9:45 ` Ville Syrjälä
  0 siblings, 2 replies; 3+ messages in thread
From: Thomas Richter @ 2015-03-28 10:07 UTC (permalink / raw)
  To: Intel Graphics Development

Hi folks,

this is a patch against drm-intel-nightly that enables an apparently 
undocumented feature of the intel VCH DVO chips. Bit 4 of the VR01 
register controls an automatic dithering for 18bpp outputs which greatly 
improves the visual image quality for 24bpp display buffers.

The bios of my IBM R31 automatically enables this feature, it is 
disabled again by the ivchr driver. The following patch checks whether 
the display is connected with a 18bpp connection and if so, re-enables 
dithering.

This feature has been tested to work fine on 16bpp images and with the 
scaler on and off. There are no ill side effects.

Please let me know whether this patch is acceptable.

Thanks and greetings,

	Thomas


Signed-off-by: Thomas Richter <thor@math.tu-berlin.de>
---
  drivers/gpu/drm/i915/dvo_ivch.c |   21 ++++++++++++++++++---
  1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ivch.c 
b/drivers/gpu/drm/i915/dvo_ivch.c
index 0f2587f..89b08a8 100644
--- a/drivers/gpu/drm/i915/dvo_ivch.c
+++ b/drivers/gpu/drm/i915/dvo_ivch.c
@@ -23,6 +23,9 @@
   * Authors:
   *    Eric Anholt <eric@anholt.net>
   *
+ * Minor modifications (Dithering enable):
+ *    Thomas Richter <thor@math.tu-berlin.de>
+ *
   */

  #include "dvo.h"
@@ -59,6 +62,8 @@
  # define VR01_DVO_BYPASS_ENABLE                (1 << 1)
  /** Enables the DVO clock */
  # define VR01_DVO_ENABLE               (1 << 0)
+/** Enable dithering for 18bpp panels. Not documented. */
+# define VR01_DITHER_ENABLE             (1 << 4)

  /*
   * LCD Interface Format
@@ -74,6 +79,8 @@
  # define VR10_INTERFACE_2X18           (2 << 2)
  /** Enables 2x24-bit LVDS output */
  # define VR10_INTERFACE_2X24           (3 << 2)
+/** Mask that defines the depth of the pipeline */
+# define VR10_INTERFACE_DEPTH_MASK      (3 << 2)

  /*
   * VR20 LCD Horizontal Display Size
@@ -342,9 +349,15 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
                           struct drm_display_mode *adjusted_mode)
  {
         uint16_t vr40 = 0;
-       uint16_t vr01;
+       uint16_t vr01 = 0;
+       uint16_t vr10;
+
+       ivch_read(dvo, VR10, &vr10);
+       /* Enable dithering for 18 bpp pipelines */
+       vr10 &= VR10_INTERFACE_DEPTH_MASK;
+       if (vr10 == VR10_INTERFACE_2X18 || vr10 == VR10_INTERFACE_1X18)
+               vr01 = VR01_DITHER_ENABLE;

-       vr01 = 0;
         vr40 = (VR40_STALL_ENABLE | VR40_VERTICAL_INTERP_ENABLE |
                 VR40_HORIZONTAL_INTERP_ENABLE);

@@ -353,7 +366,7 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
                 uint16_t x_ratio, y_ratio;

                 vr01 |= VR01_PANEL_FIT_ENABLE;
-               vr40 |= VR40_CLOCK_GATING_ENABLE;
+               vr40 |= VR40_CLOCK_GATING_ENABLE | 
VR40_ENHANCED_PANEL_FITTING;
                 x_ratio = (((mode->hdisplay - 1) << 16) /
                            (adjusted_mode->hdisplay - 1)) >> 2;
                 y_ratio = (((mode->vdisplay - 1) << 16) /
@@ -380,6 +393,8 @@ static void ivch_dump_regs(struct intel_dvo_device *dvo)
         DRM_DEBUG_KMS("VR00: 0x%04x\n", val);
         ivch_read(dvo, VR01, &val);
         DRM_DEBUG_KMS("VR01: 0x%04x\n", val);
+       ivch_read(dvo, VR10, &val);
+       DRM_DEBUG_KMS("VR10: 0x%04x\n", val);
         ivch_read(dvo, VR30, &val);
         DRM_DEBUG_KMS("VR30: 0x%04x\n", val);
         ivch_read(dvo, VR40, &val);
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] Enable dithering on intel VCH DVO chips on 18bpp panels
  2015-03-28 10:07 [PATCH] Enable dithering on intel VCH DVO chips on 18bpp panels Thomas Richter
@ 2015-03-30  9:37 ` Daniel Vetter
  2015-03-30  9:45 ` Ville Syrjälä
  1 sibling, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2015-03-30  9:37 UTC (permalink / raw)
  To: Thomas Richter; +Cc: Intel Graphics Development

On Sat, Mar 28, 2015 at 11:07:09AM +0100, Thomas Richter wrote:
> Hi folks,
> 
> this is a patch against drm-intel-nightly that enables an apparently
> undocumented feature of the intel VCH DVO chips. Bit 4 of the VR01 register
> controls an automatic dithering for 18bpp outputs which greatly improves the
> visual image quality for 24bpp display buffers.
> 
> The bios of my IBM R31 automatically enables this feature, it is disabled
> again by the ivchr driver. The following patch checks whether the display is
> connected with a 18bpp connection and if so, re-enables dithering.
> 
> This feature has been tested to work fine on 16bpp images and with the
> scaler on and off. There are no ill side effects.
> 
> Please let me know whether this patch is acceptable.
> 
> Thanks and greetings,
> 
> 	Thomas
> 
> 
> Signed-off-by: Thomas Richter <thor@math.tu-berlin.de>

Patch looks good but seems mangled. Can you please resend using git
send-email? That tool takes care of all the details. Also if you want to
add a few things which should not be part of the commit message you can
put them below the the s-o-b line like this:

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
<Further explanations and context goes here>

Cheers, Daniel

git apply-mbox will the automatically cut away everything below the ---
line and only take the text above it as the commit message.

Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/dvo_ivch.c |   21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/dvo_ivch.c
> b/drivers/gpu/drm/i915/dvo_ivch.c
> index 0f2587f..89b08a8 100644
> --- a/drivers/gpu/drm/i915/dvo_ivch.c
> +++ b/drivers/gpu/drm/i915/dvo_ivch.c
> @@ -23,6 +23,9 @@
>   * Authors:
>   *    Eric Anholt <eric@anholt.net>
>   *
> + * Minor modifications (Dithering enable):
> + *    Thomas Richter <thor@math.tu-berlin.de>
> + *
>   */
> 
>  #include "dvo.h"
> @@ -59,6 +62,8 @@
>  # define VR01_DVO_BYPASS_ENABLE                (1 << 1)
>  /** Enables the DVO clock */
>  # define VR01_DVO_ENABLE               (1 << 0)
> +/** Enable dithering for 18bpp panels. Not documented. */
> +# define VR01_DITHER_ENABLE             (1 << 4)
> 
>  /*
>   * LCD Interface Format
> @@ -74,6 +79,8 @@
>  # define VR10_INTERFACE_2X18           (2 << 2)
>  /** Enables 2x24-bit LVDS output */
>  # define VR10_INTERFACE_2X24           (3 << 2)
> +/** Mask that defines the depth of the pipeline */
> +# define VR10_INTERFACE_DEPTH_MASK      (3 << 2)
> 
>  /*
>   * VR20 LCD Horizontal Display Size
> @@ -342,9 +349,15 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
>                           struct drm_display_mode *adjusted_mode)
>  {
>         uint16_t vr40 = 0;
> -       uint16_t vr01;
> +       uint16_t vr01 = 0;
> +       uint16_t vr10;
> +
> +       ivch_read(dvo, VR10, &vr10);
> +       /* Enable dithering for 18 bpp pipelines */
> +       vr10 &= VR10_INTERFACE_DEPTH_MASK;
> +       if (vr10 == VR10_INTERFACE_2X18 || vr10 == VR10_INTERFACE_1X18)
> +               vr01 = VR01_DITHER_ENABLE;
> 
> -       vr01 = 0;
>         vr40 = (VR40_STALL_ENABLE | VR40_VERTICAL_INTERP_ENABLE |
>                 VR40_HORIZONTAL_INTERP_ENABLE);
> 
> @@ -353,7 +366,7 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
>                 uint16_t x_ratio, y_ratio;
> 
>                 vr01 |= VR01_PANEL_FIT_ENABLE;
> -               vr40 |= VR40_CLOCK_GATING_ENABLE;
> +               vr40 |= VR40_CLOCK_GATING_ENABLE |
> VR40_ENHANCED_PANEL_FITTING;
>                 x_ratio = (((mode->hdisplay - 1) << 16) /
>                            (adjusted_mode->hdisplay - 1)) >> 2;
>                 y_ratio = (((mode->vdisplay - 1) << 16) /
> @@ -380,6 +393,8 @@ static void ivch_dump_regs(struct intel_dvo_device *dvo)
>         DRM_DEBUG_KMS("VR00: 0x%04x\n", val);
>         ivch_read(dvo, VR01, &val);
>         DRM_DEBUG_KMS("VR01: 0x%04x\n", val);
> +       ivch_read(dvo, VR10, &val);
> +       DRM_DEBUG_KMS("VR10: 0x%04x\n", val);
>         ivch_read(dvo, VR30, &val);
>         DRM_DEBUG_KMS("VR30: 0x%04x\n", val);
>         ivch_read(dvo, VR40, &val);
> --
> 1.7.10.4

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] Enable dithering on intel VCH DVO chips on 18bpp panels
  2015-03-28 10:07 [PATCH] Enable dithering on intel VCH DVO chips on 18bpp panels Thomas Richter
  2015-03-30  9:37 ` Daniel Vetter
@ 2015-03-30  9:45 ` Ville Syrjälä
  1 sibling, 0 replies; 3+ messages in thread
From: Ville Syrjälä @ 2015-03-30  9:45 UTC (permalink / raw)
  To: Thomas Richter; +Cc: Intel Graphics Development

On Sat, Mar 28, 2015 at 11:07:09AM +0100, Thomas Richter wrote:
> Hi folks,
> 
> this is a patch against drm-intel-nightly that enables an apparently 
> undocumented feature of the intel VCH DVO chips. Bit 4 of the VR01 
> register controls an automatic dithering for 18bpp outputs which greatly 
> improves the visual image quality for 24bpp display buffers.
> 
> The bios of my IBM R31 automatically enables this feature, it is 
> disabled again by the ivchr driver. The following patch checks whether 
> the display is connected with a 18bpp connection and if so, re-enables 
> dithering.
> 
> This feature has been tested to work fine on 16bpp images and with the 
> scaler on and off. There are no ill side effects.
> 
> Please let me know whether this patch is acceptable.
> 
> Thanks and greetings,
> 
> 	Thomas
> 
> 
> Signed-off-by: Thomas Richter <thor@math.tu-berlin.de>
> ---
>   drivers/gpu/drm/i915/dvo_ivch.c |   21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/dvo_ivch.c 
> b/drivers/gpu/drm/i915/dvo_ivch.c
> index 0f2587f..89b08a8 100644
> --- a/drivers/gpu/drm/i915/dvo_ivch.c
> +++ b/drivers/gpu/drm/i915/dvo_ivch.c
> @@ -23,6 +23,9 @@
>    * Authors:
>    *    Eric Anholt <eric@anholt.net>
>    *
> + * Minor modifications (Dithering enable):
> + *    Thomas Richter <thor@math.tu-berlin.de>
> + *
>    */
> 
>   #include "dvo.h"
> @@ -59,6 +62,8 @@
>   # define VR01_DVO_BYPASS_ENABLE                (1 << 1)
>   /** Enables the DVO clock */
>   # define VR01_DVO_ENABLE               (1 << 0)
> +/** Enable dithering for 18bpp panels. Not documented. */
> +# define VR01_DITHER_ENABLE             (1 << 4)
> 
>   /*
>    * LCD Interface Format
> @@ -74,6 +79,8 @@
>   # define VR10_INTERFACE_2X18           (2 << 2)
>   /** Enables 2x24-bit LVDS output */
>   # define VR10_INTERFACE_2X24           (3 << 2)
> +/** Mask that defines the depth of the pipeline */
> +# define VR10_INTERFACE_DEPTH_MASK      (3 << 2)
> 
>   /*
>    * VR20 LCD Horizontal Display Size
> @@ -342,9 +349,15 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
>                            struct drm_display_mode *adjusted_mode)
>   {
>          uint16_t vr40 = 0;
> -       uint16_t vr01;
> +       uint16_t vr01 = 0;
> +       uint16_t vr10;
> +
> +       ivch_read(dvo, VR10, &vr10);
> +       /* Enable dithering for 18 bpp pipelines */
> +       vr10 &= VR10_INTERFACE_DEPTH_MASK;
> +       if (vr10 == VR10_INTERFACE_2X18 || vr10 == VR10_INTERFACE_1X18)
> +               vr01 = VR01_DITHER_ENABLE;
> 
> -       vr01 = 0;
>          vr40 = (VR40_STALL_ENABLE | VR40_VERTICAL_INTERP_ENABLE |
>                  VR40_HORIZONTAL_INTERP_ENABLE);
> 
> @@ -353,7 +366,7 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
>                  uint16_t x_ratio, y_ratio;
> 
>                  vr01 |= VR01_PANEL_FIT_ENABLE;
> -               vr40 |= VR40_CLOCK_GATING_ENABLE;
> +               vr40 |= VR40_CLOCK_GATING_ENABLE | 
> VR40_ENHANCED_PANEL_FITTING;

You're also enabling this other bit. The spec isn't very clear on most
things, but I might assume this enables the higher order upscale filter.
I don't see any programmable parameters for the filter in spec, so it
might be safe to just enable it always. But yeah, not really sure about
that. OTOH I don't expect this DVO chip to be in use in many other
machines, so chances of breaking some other machine seem rather low.

However it would seems better if this was a separate patch, or of it's
really just a part of getting the dither bit to work then it would be
good to at least mention that fact in the commit message, or maybe
add a comment into the code explaining the relationship of the two bits
(assuming there is one).

>                  x_ratio = (((mode->hdisplay - 1) << 16) /
>                             (adjusted_mode->hdisplay - 1)) >> 2;
>                  y_ratio = (((mode->vdisplay - 1) << 16) /
> @@ -380,6 +393,8 @@ static void ivch_dump_regs(struct intel_dvo_device *dvo)
>          DRM_DEBUG_KMS("VR00: 0x%04x\n", val);
>          ivch_read(dvo, VR01, &val);
>          DRM_DEBUG_KMS("VR01: 0x%04x\n", val);
> +       ivch_read(dvo, VR10, &val);
> +       DRM_DEBUG_KMS("VR10: 0x%04x\n", val);
>          ivch_read(dvo, VR30, &val);
>          DRM_DEBUG_KMS("VR30: 0x%04x\n", val);
>          ivch_read(dvo, VR40, &val);
> --
> 1.7.10.4

-- 
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] 3+ messages in thread
end of thread, other threads:[~2015-03-30  9:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-28 10:07 [PATCH] Enable dithering on intel VCH DVO chips on 18bpp panels Thomas Richter
2015-03-30  9:37 ` Daniel Vetter
2015-03-30  9:45 ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox