| Message ID | 55167D4D.5030700@math.tu-berlin.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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;
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(-) 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