Re: [Intel-gfx] [PATCH] Implement Limited Video Range

[Peter Frühberger]

Thanks for your comments, I tried to implement a "flag driven version"
below.

On Fri, Nov 11, 2016 at 2:57 PM, Ville Syrjälä <
ville.syrj...@linux.intel.com> wrote:

> On Fri, Nov 11, 2016 at 09:53:35AM +0100, Peter Frühberger wrote:
> > Hi,
> >
> > I was implementing this suggestion today and I think that will confuse
> > users and also devs maintaining that code. Out of the following reason:
> > compress_color_range can be true or false, it does not reference a mode,
> > but an additional setting that only influences color clamping / scaling.
> >
> > What do we expect if someone runs in Full Range and has
> > compress_color_range set to true? Also what do we expect if someone runs
> in
> > Limited Range and additionally set this flag to true? In some cases it
> > would do nothing and was not transparent.
>
> I didn't mean you should add a new property for this. Just an internal
> flag.
> [...]
>

In my opinion that makes maintainability much harder. Looking forward to
your comments.


>From f3bccf1611108247add0d85e8faed5430971cd71 Mon Sep 17 00:00:00 2001
From: fritsch <peter.fruehber...@gmail.com>
Date: Sat, 16 Sep 2017 13:54:06 +0200
Subject: [PATCH] i915: Implement uncompressed Video Range output

---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_color.c   | 12 
 drivers/gpu/drm/i915/intel_display.c |  6 --
 drivers/gpu/drm/i915/intel_dp.c  |  3 ++-
 drivers/gpu/drm/i915/intel_drv.h | 11 +--
 drivers/gpu/drm/i915/intel_hdmi.c| 16 +---
 drivers/gpu/drm/i915/intel_modes.c   |  1 +
 7 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5aecbf795b55..70bd525317c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4230,6 +4230,7 @@ __raw_write(64, q)
 #define INTEL_BROADCAST_RGB_AUTO 0
 #define INTEL_BROADCAST_RGB_FULL 1
 #define INTEL_BROADCAST_RGB_LIMITED 2
+#define INTEL_BROADCAST_RGB_VIDEO 3

 static inline i915_reg_t i915_vgacntrl_reg(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
index ff9ecd211abb..b72477a43ea4 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -149,7 +149,8 @@ static void i9xx_load_csc_matrix(struct
drm_crtc_state *crtc_state)
(struct drm_color_ctm *)crtc_state->ctm->data;
uint64_t input[9] = { 0, };

-   if (intel_crtc_state->limited_color_range) {
+   if (intel_crtc_state->limited_color_range &&
+   intel_crtc_state->compress_range) {
ctm_mult_by_limited(input, ctm->matrix);
} else {
for (i = 0; i < ARRAY_SIZE(input); i++)
@@ -201,7 +202,8 @@ static void i9xx_load_csc_matrix(struct
drm_crtc_state *crtc_state)
 * into consideration.
 */
for (i = 0; i < 3; i++) {
-   if (intel_crtc_state->limited_color_range)
+   if (intel_crtc_state->limited_color_range &&
+   intel_crtc_state->compress_range)
coeffs[i * 3 + i] =
I9XX_CSC_COEFF_LIMITED_RANGE;
else
@@ -225,7 +227,8 @@ static void i9xx_load_csc_matrix(struct
drm_crtc_state *crtc_state)
if (INTEL_GEN(dev_priv) > 6) {
uint16_t postoff = 0;

-   if (intel_crtc_state->limited_color_range)
+   if (intel_crtc_state->limited_color_range &&
+   intel_crtc_state->compress_range)
postoff = (16 * (1 << 12) / 255) & 0x1fff;

I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff);
@@ -236,7 +239,8 @@ static void i9xx_load_csc_matrix(struct
drm_crtc_state *crtc_state)
} else {
uint32_t mode = CSC_MODE_YUV_TO_RGB;

-   if (intel_crtc_state->limited_color_range)
+   if (intel_crtc_state->limited_color_range &&
+   intel_crtc_state->compress_range)
mode |= CSC_BLACK_SCREEN_OFFSET;

I915_WRITE(PIPE_CSC_MODE(pipe), mode);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8599e425abb1..4de43eb12f0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7247,7 +7247,8 @@ static void i9xx_set_pipeconf(struct intel_crtc
*intel_crtc)
pipeconf |= PIPECONF_PROGRESSIVE;

if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&

Re: [Intel-gfx] [PATCH] drm/i915/dp: Stop enabling limited color ranges for everything

[Peter Frühberger]

Hi,

On Thu, Jan 5, 2017 at 9:52 AM, Daniel Vetter  wrote:

> On Thu, Jan 05, 2017 at 10:41:07AM +0200, Jani Nikula wrote:
> > No matter what we do here, the question remains what to do with
> > Chamelium. Changing the color range is really a workaround for
> > Chamelium, not a fix. Using CEA range is perfectly fine per DP spec.
>
> Can we just set a non-CEA mode/edid for chamelium, problem solved? We want
> to do that anyway for HDMI, where you really have to do the limited range
> dance to make stuff display correctly.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

I shortly want to emphasize on this:
https://lists.freedesktop.org/archives/intel-gfx/2015-November/081516.html

No matter what you sync with the Port - userspace must have a chance to
know what happens with its DATA. It is a huge problem when the driver
clamps by itself assuming everything it gets would be full range. Or when
the Output device clamps and does not scale, then you loose, e.g. 0 ..16
and 235.. 255. On the other hand such scenarios make userspace scale data -
especially video data - twice. One time to full range and later down in the
driver when using CEA mode, e.g. Limited 16:235.

Please keep that in mind. It's problematic already nowadays when a 1:1
output without color altering is high importance.

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

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Peter Frühberger]

Forgot to CC the list, sorry.

On Wed, Jan 4, 2017 at 11:42 AM, Peter Frühberger <frit...@kodi.tv> wrote:

> Hi Jani,
> thanks for your reply
>
> On Wed, Jan 4, 2017 at 10:34 AM, Jani Nikula <jani.nik...@intel.com>
> wrote:
>
>> On Wed, 04 Jan 2017, Peter Frühberger <frit...@kodi.tv> wrote:
>> > Hi
>> >
>> > On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran <
>> > dhinakaran.pandi...@intel.com> wrote:
>> >
>> >> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
>> >> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
>> >> dhinakaran.pandi...@intel.com> wrote:
>> >> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
>> >> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
>> >> dhinakaran.pandi...@intel.com> wrote:
>> >> > >> > Enabling DP audio stall fix is necessary to play audio over DP
>> >> HBR2. So,
>> >> > >> > let's set this bit right before enabling the audio codec.
>> Playing
>> >> audio
>> >> > >> > without setting this bit results in pipe FIFO underruns.
>> >> > >> >
>> >> > >> > This workaround is applicable only for audio sample rates up to
>> >> 96kHz. For
>> >> > >> > frequencies above 96kHz, this is insufficient and cdclk should
>> be
>> >> increased
>> >> > >> > to at least 432 MHz, just like BDW. Since, the audio driver
>> does not
>> >> > >> > support sample rates > 48 kHz, we are safe with this fix for
>> now.
>> >> > >>
>> >> > >> Do we still need this patch now that these two have been pushed?
>> >> > >>
>> >> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
>> >> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9
>> >> platforms
>> >> > >>
>> >> > >> BR,
>> >> > >> Jani.
>> >> > >>
>> >> > >>
>> >> > >>
>> >> > >
>> >> > > No, we are good afaik. This patch would have helped us to make use
>> of a
>> >> > > lower cdclk (337.5 MHz), with constraints on audio bit rate.
>> Operating
>> >> > > at 432 MHz, like we do now, rules out the need for this patch.
>> >> >
>> >> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
>> >> >
>> >> > BR,
>> >> > Jani.
>> >> >
>> >>
>> >> Good point, I think it will depend on the audio sampling rate. But, I
>> >> have to figure out a way to play high sampling rate audio (> 96 KHz)
>> and
>> >> test 5.4 Gbps with 1 or 2 lanes.
>> >>
>> >> The other option is to play safe and apply this patch with even lesser
>> >> restrictions, say link rate >= 2.7 Gbps.
>> >>
>> >>
>> >> -DK
>> >>
>> >
>> > as we are currently talking about high samplerates in this context. I
>> > wanted to post a perhaps related issue. On my Apollo Lake (J4205) I have
>> > two outputs. One DVI and one HDMI 2.0 via internal DP. Via DVI the
>> > following works without issues, via DP it fails. As the original commit
>> > mentions HBR, I think there is still something missing. We submit
>> TrueHD,
>> > DTS-HD via 192 khz and 16 bit format while setting AES0=2
>> >
>> > You can easily reproduce with (you obviously need a DTS-HD, TrueHD
>> capable
>> > AVR attached to your HDMI 2.0 (DP) out):
>>
>> Just to clarify, is the DP -> HDMI2.0 converter internal to the machine?
>> LSPCON related messages in the dmesg with drm.debug=14? Do you have a DP
>> or an HDMI physical connector in the chassis?
>>
>
> The chip used is: https://media.digikey.com/pdf/Data%20Sheets/MegaChips%
> 20PDFs/MCDP28x0_Datasheet.pdf which is the same on all intel nucs,
> including the new Kabilake ones. So it's internal.
> Mainboard: http://www.asrock.com/MB/Intel/J4205-ITX/index.us.asp (DVI-D,
> HDMI (2.0 via this above chip), VGA)
> Here is the output of the a boot up with drm.debug=14:
> http://paste.ubuntu.com/23738282/
> (I used a pastebin site to not spam the ML, is that okay for the future?)
>
> Best regards
> Peter
>
>
>>
>>
>> BR,
>> Jani.
>

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

[Peter Frühberger]

Hi

On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran <
dhinakaran.pandi...@intel.com> wrote:

> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
> dhinakaran.pandi...@intel.com> wrote:
> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
> dhinakaran.pandi...@intel.com> wrote:
> > >> > Enabling DP audio stall fix is necessary to play audio over DP
> HBR2. So,
> > >> > let's set this bit right before enabling the audio codec. Playing
> audio
> > >> > without setting this bit results in pipe FIFO underruns.
> > >> >
> > >> > This workaround is applicable only for audio sample rates up to
> 96kHz. For
> > >> > frequencies above 96kHz, this is insufficient and cdclk should be
> increased
> > >> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > >> > support sample rates > 48 kHz, we are safe with this fix for now.
> > >>
> > >> Do we still need this patch now that these two have been pushed?
> > >>
> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9
> platforms
> > >>
> > >> BR,
> > >> Jani.
> > >>
> > >>
> > >>
> > >
> > > No, we are good afaik. This patch would have helped us to make use of a
> > > lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
> > > at 432 MHz, like we do now, rules out the need for this patch.
> >
> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
> >
> > BR,
> > Jani.
> >
>
> Good point, I think it will depend on the audio sampling rate. But, I
> have to figure out a way to play high sampling rate audio (> 96 KHz) and
> test 5.4 Gbps with 1 or 2 lanes.
>
> The other option is to play safe and apply this patch with even lesser
> restrictions, say link rate >= 2.7 Gbps.
>
>
> -DK
>

as we are currently talking about high samplerates in this context. I
wanted to post a perhaps related issue. On my Apollo Lake (J4205) I have
two outputs. One DVI and one HDMI 2.0 via internal DP. Via DVI the
following works without issues, via DP it fails. As the original commit
mentions HBR, I think there is still something missing. We submit TrueHD,
DTS-HD via 192 khz and 16 bit format while setting AES0=2

You can easily reproduce with (you obviously need a DTS-HD, TrueHD capable
AVR attached to your HDMI 2.0 (DP) out):

#TrueHD
aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
testi.truehd.anssi1.ff.60s.spdif
#DTS-HD
aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
testi.dtshd.anssi1.ma-71-24.spdif
Samples:
http://www.avenard.org/files/media/mediatest/audiotest/HDAUDIO/Passthrough/

For the old HDMI 1.x chips it was fixed via:
https://bugs.freedesktop.org/show_bug.cgi?id=49055

Is this also planned for DP within that patch series?

Best regards
Peter

>
> > >
> > > -DK
> > >
> > >> >
> > >> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > >> > Fixed the port clock typo
> > >> > Added TODO comment
> > >> > Signed-off-by: Dhinakaran Pandiyan 
> > >> > ---
> > >> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
> > >> >  drivers/gpu/drm/i915/intel_audio.c | 30
> +-
> > >> >  2 files changed, 34 insertions(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > >> > index 00efaa1..76dac48 100644
> > >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > >> > @@ -6236,6 +6236,11 @@ enum {
> > >> >  #define SLICE_ECO_CHICKEN0  _MMIO(0x7308)
> > >> >  #define   PIXEL_MASK_CAMMING_DISABLE(1 << 14)
> > >> >
> > >> > +#define _CHICKEN_TRANS_A0x420C0
> > >> > +#define _CHICKEN_TRANS_B0x420C4
> > >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A,
> _CHICKEN_TRANS_B)
> > >> > +#define SPARE_13(1<<13)
> > >> > +
> > >> >  /* WaCatErrorRejectionIssue */
> > >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG  _MMIO(0x9030)
> > >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB   (1<<11)
> > >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > >> > index 7093cfb..894f11e 100644
> > >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct
> intel_encoder *encoder)
> > >> >  {
> > >> >  struct drm_i915_private *dev_priv =
> to_i915(encoder->base.dev);
> > >> >  struct intel_crtc *intel_crtc =
> to_intel_crtc(encoder->base.crtc);
> > >> > +struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > >> > +enum transcoder cpu_transcoder =
> crtc_config->cpu_transcoder;
> > >> >  enum pipe pipe = intel_crtc->pipe;
> > >> >  uint32_t tmp;
> > >> >
> > >> > @@ -290,13 +292,21 @@ static void 

Re: [Intel-gfx] [PATCH] Implement Limited Video Range

[Peter Frühberger]

Hi,

I was implementing this suggestion today and I think that will confuse
users and also devs maintaining that code. Out of the following reason:
compress_color_range can be true or false, it does not reference a mode,
but an additional setting that only influences color clamping / scaling.

What do we expect if someone runs in Full Range and has
compress_color_range set to true? Also what do we expect if someone runs in
Limited Range and additionally set this flag to true? In some cases it
would do nothing and was not transparent.

In my original patch I implemented a new mode, which was: Video Range
16:235. That meant: Tell Limited info frame, don't alter any colors. It was
a combination of two things: limited info frame + compress_color_range =
false;

The driver code mainly uses: intel_hdmi->limited_color_range to distinguish
if colors should be clamped or not. Therefore if the above value was set,
that just needed setting to false.

I think if we decide for a compress_color_range bool, it should not work as
an entire new mode but as an option to alter current mode, meaning
something not set via "BroadCast RGB".

Any thoughts on that matter?

On Wed, Nov 9, 2016 at 9:25 PM, Ville Syrjälä <ville.syrj...@linux.intel.com
> wrote:

> On Wed, Nov 09, 2016 at 09:11:58PM +0100, Peter Frühberger wrote:
> > I am currently not sure what the way forward should be.
> >
> > We are successfully implementing this patch in e.g. LibreELEC an embedded
> > distribution for home theater pcs. But through current bug reports - I am
> > not sure, if we should give such functionality to the user, that is
> > overwritten on a lower level, e.g. it does not what the user expects.
> >
> > E.g. since some time displays are driven with 10 or 12 bit from the intel
> > driver, image data is dithered, scaled back and forth without being
> > transparent for the user. In fact we have some users with hdmi capture
> > cards, coming back to us if some decoded values of their video stuff does
> > not match.
>
> Re-reading what was written (can't remember anymore obviously), I think
> what both me and Daniel were after was a cleaner separation of the
> "what should the infoframe say?" and "how should we mangle the color
> data?".
>
> Daniel's "limit_color_range" is a wee bit too similar looking to
> "limited_color_range" for my liking. So how about we go with something
> like "bool compress_color_range". It also conveys the fact that we're
> not just clamping things. Any objections/thoughts/better ideas?
>
> >
> > Best regards
> > Peter
> >
> > On Mon, Nov 30, 2015 at 3:11 PM, Ville Syrjälä <
> > ville.syrj...@linux.intel.com> wrote:
> >
> > > On Mon, Nov 30, 2015 at 09:43:32AM +0100, Daniel Vetter wrote:
> > > > On Mon, Nov 30, 2015 at 08:08:48AM +0100, Peter Frühberger wrote:
> > > > > (Resent cause of moderation)
> > > > >
> > > > > This implements a highly needed feature in a minimal non
> instructive
> > > way.
> > > > > Consider a Limited Range display (like most TVs) where you want to
> > > watch a
> > > > > decoded video. The TV is set to limited range and the intel driver
> also
> > > > > uses full scaling Limited 16:235 mode, e.g. if you display the gray
> > > value
> > > > > 16 - the driver will postprocess it to something like ~ 22.
> > > > >
> > > > > The following happens currently in linux intel video world:
> > > > > Video is decoded with e.g. vaapi, the decoded surface is either
> used
> > > via
> > > > > EGL directly in Limited Range. Limited Range processing takes place
> > > and at
> > > > > the end while rendering the intel driver will scale down the
> limited
> > > range
> > > > > data again as it cannot distunguish of the data it gets when e.g.
> > > rendering
> > > > > with OpenGL like we succesfully do in kodi.
> > > > >
> > > > > Another use case is vaapi when using the old vaCopySurfaceGLX
> > > > > (vaPutSurface) which would automatically use BT601 / BT709 matrix
> to
> > > > > upscale the content without any dithering to Full Range RGB. Later
> on
> > > this
> > > > > is scaled down again with the Limited 16:235 setting and output.
> > > Quality
> > > > > degrades two times.
> > > > >
> > > > > Users and applications widely used want to make sure that colors
> > > appear on
> > > > > screen like they were processed after t

Re: [Intel-gfx] [PATCH] Implement Limited Video Range

[Peter Frühberger]

I am currently not sure what the way forward should be.

We are successfully implementing this patch in e.g. LibreELEC an embedded
distribution for home theater pcs. But through current bug reports - I am
not sure, if we should give such functionality to the user, that is
overwritten on a lower level, e.g. it does not what the user expects.

E.g. since some time displays are driven with 10 or 12 bit from the intel
driver, image data is dithered, scaled back and forth without being
transparent for the user. In fact we have some users with hdmi capture
cards, coming back to us if some decoded values of their video stuff does
not match.

Best regards
Peter

On Mon, Nov 30, 2015 at 3:11 PM, Ville Syrjälä <
ville.syrj...@linux.intel.com> wrote:

> On Mon, Nov 30, 2015 at 09:43:32AM +0100, Daniel Vetter wrote:
> > On Mon, Nov 30, 2015 at 08:08:48AM +0100, Peter Frühberger wrote:
> > > (Resent cause of moderation)
> > >
> > > This implements a highly needed feature in a minimal non instructive
> way.
> > > Consider a Limited Range display (like most TVs) where you want to
> watch a
> > > decoded video. The TV is set to limited range and the intel driver also
> > > uses full scaling Limited 16:235 mode, e.g. if you display the gray
> value
> > > 16 - the driver will postprocess it to something like ~ 22.
> > >
> > > The following happens currently in linux intel video world:
> > > Video is decoded with e.g. vaapi, the decoded surface is either used
> via
> > > EGL directly in Limited Range. Limited Range processing takes place
> and at
> > > the end while rendering the intel driver will scale down the limited
> range
> > > data again as it cannot distunguish of the data it gets when e.g.
> rendering
> > > with OpenGL like we succesfully do in kodi.
> > >
> > > Another use case is vaapi when using the old vaCopySurfaceGLX
> > > (vaPutSurface) which would automatically use BT601 / BT709 matrix to
> > > upscale the content without any dithering to Full Range RGB. Later on
> this
> > > is scaled down again with the Limited 16:235 setting and output.
> Quality
> > > degrades two times.
> > >
> > > Users and applications widely used want to make sure that colors
> appear on
> > > screen like they were processed after the last processing step. In
> video
> > > case two use cases make sense:
> > >
> > > a) scaling limited range to full range with dithering applied when you
> need
> > > to output fullrange
> > > b) already having limited range and outputting that without any further
> > > touching by the driver
> > >
> > > Use case a) is already possible. Usecase b) will be possible with the
> > > attached patch. It is a possibility to get Limited Range on screen in
> > > perfect quality in 2k15.
> > >
> > > For the future a userspace api to provide info frames and so on in a
> > > generic way should be considered but until we have this I bet we have 2
> > > years to go. This solution also works on X11 (without wayland) and is
> > > useful for existing applications.
> > >
> > > Thanks much for consideration.
> > >
> > > ---
> > > From deaa9d730c08aefefe3671d073d93d970bb39a31 Mon Sep 17 00:00:00 2001
> > > From: fritsch <peter.fruehber...@gmail.com>
> > > Date: Sun, 29 Nov 2015 16:38:14 +0100
> > > Subject: [PATCH] Intel: Implement Video Color Range
> > >
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> > >  drivers/gpu/drm/i915/intel_display.c |  4 ++--
> > >  drivers/gpu/drm/i915/intel_drv.h |  8 
> > >  drivers/gpu/drm/i915/intel_hdmi.c| 17 +++--
> > >  drivers/gpu/drm/i915/intel_modes.c   |  1 +
> > >  5 files changed, 27 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 95bb27d..e453461 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -3449,6 +3449,7 @@ int intel_freq_opcode(struct drm_i915_private
> > > *dev_priv, int val);
> > >  #define INTEL_BROADCAST_RGB_AUTO 0
> > >  #define INTEL_BROADCAST_RGB_FULL 1
> > >  #define INTEL_BROADCAST_RGB_LIMITED 2
> > > +#define INTEL_BROADCAST_RGB_VIDEO 3
> > >
> > >  static inline uint32_t i915_vgacntrl_reg(struct drm_device *dev)
> > >  {
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > >

Re: [Intel-gfx] drm/i915 4.5/4.6 stable backport request for CHV

[Peter Frühberger]

Hi guys,

2016-06-06 11:32 GMT+02:00 Ville Syrjälä :

> On Sat, Jun 04, 2016 at 02:06:58PM -0700, Greg KH wrote:
> > On Fri, May 27, 2016 at 11:30:30AM +0300, ville.syrj...@linux.intel.com
> wrote:
> > > From: Ville Syrjälä 
> > >
> > > Several nasty i915 regressions affecting CHV slipped through
> > > to 4.5 and 4.6.
> > >
> > > The first fix we want in 4.5 and 4.6 is
> > > commit caed361d83b2 ("drm/i915: Fix watermarks for VLV/CHV")
> > > It won't cherry-pick cleanly to either one, so I've included conflict
> > > free versions for both. This one fixes display FIFO underruns that can
> > > lead to the screen totally blanking out.
> >
> > Now applied, thanks.
> >
> > > The other one I'd like to have in 4.6 is
> > > commit 9f6151c90390 ("drm/i915: Pass the correct crtc state to
> .update_plane()")
> > > which avoids a totally corrupted display in some cases.
> >
> > Now applied.
> >
> > > And the third on is a bit more annoying. The regression is caused by
> > > commit 9dbaab56ac09 ("drm/i915: Exit cherryview_irq_handler() after
> one pass")
> > > which I though we had prevented from getting out on its own, but turns
> > > out I was wrong. It basically makes the GPU unusable, so we do need to
> > > fix it somehow. The simple solution would be to revert it in 4.6 only.
> > > The more complicated solution is to backport the proper fix, which more
> > > or less requires the following set of commits [1], which is maybe a bit
> > > too much for stable. I could try to trim it a bit perhaps, but then we
> > > start to enter the territory of untested code which I don't
> particularly
> > > like. Let me know what you think.
> > >
> > > [1]
> > >  1e1cace942ef ("drm/i915: Eliminate loop from VLV irq handler")
> > >  a5e485a95c9c ("drm/i915: Clear VLV_IER around irq processing")
> > >  4a0a0202b023 ("drm/i915: Clear VLV_MASTER_IER around irq processing")
> > >  7ce4d1f2730f ("drm/i915: Clear VLV_IIR after PIPESTAT")
> > >  34c7b8a7b8b5 ("drm/i915: Set up VLV_MASTER_IER consistently")
> > >  e5328c43d46e ("drm/i915: Use GEN8_MASTER_IRQ_CONTROL consistently")
> > >  71b8b41d5b35 ("drm/i915: Move DPINVGTT setup to
> vlv_display_irq_reset()")
> > >  6b7eafc1b43d ("drm/i915: Warn if irq_mask isn't ~0 during vlv/cvh
> display irq postinstall")
> > >  9ab981f22bef ("drm/i915: Use GEN5_IRQ_INIT() in
> vlv_display_irq_postinstall()")
> > >  d6c698035892 ("drm/i915: Clear display interrupt before enabling when
> turning on the power well")
> > >  8bb613068a63 ("drm/i915: Move vlv/chv display irq code to a more
> logical place")
> > >  9918271efc7a ("drm/i915: Skip display irq setup if display irqs
> aren't flagged as enabled")
> > >  ad22d10654ea ("drm/i915: Fix up vlv/chv display irq setup")
> > >  93de68f94081 ("drm/i915: Remove "VLV magic" from irq setup")
> >
> > I think reverting that one patch for 4.6 makes more sense than adding
> > all of these patches.  I'll do that if you want me to.
>
> That works for me. Thanks.
>
> --
> Ville Syrjälä
> Intel OTC


Sorry for bothering you. Will the revert make it into 4.6.3? Without it BSW
on 4.6 is not really usable.

Thanks much in advance
Peter



-- 
   Key-ID: 0x1A995A9B
   keyserver: pgp.mit.edu
==
Fingerprint: 4606 DA19 EC2E 9A0B 0157  C81B DA07 CF63 1A99 5A9B
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] Implement Limited Video Range

[Peter Frühberger]

(Resent cause of moderation)

This implements a highly needed feature in a minimal non instructive way.
Consider a Limited Range display (like most TVs) where you want to watch a
decoded video. The TV is set to limited range and the intel driver also
uses full scaling Limited 16:235 mode, e.g. if you display the gray value
16 - the driver will postprocess it to something like ~ 22.

The following happens currently in linux intel video world:
Video is decoded with e.g. vaapi, the decoded surface is either used via
EGL directly in Limited Range. Limited Range processing takes place and at
the end while rendering the intel driver will scale down the limited range
data again as it cannot distunguish of the data it gets when e.g. rendering
with OpenGL like we succesfully do in kodi.

Another use case is vaapi when using the old vaCopySurfaceGLX
(vaPutSurface) which would automatically use BT601 / BT709 matrix to
upscale the content without any dithering to Full Range RGB. Later on this
is scaled down again with the Limited 16:235 setting and output. Quality
degrades two times.

Users and applications widely used want to make sure that colors appear on
screen like they were processed after the last processing step. In video
case two use cases make sense:

a) scaling limited range to full range with dithering applied when you need
to output fullrange
b) already having limited range and outputting that without any further
touching by the driver

Use case a) is already possible. Usecase b) will be possible with the
attached patch. It is a possibility to get Limited Range on screen in
perfect quality in 2k15.

For the future a userspace api to provide info frames and so on in a
generic way should be considered but until we have this I bet we have 2
years to go. This solution also works on X11 (without wayland) and is
useful for existing applications.

Thanks much for consideration.

---
>From deaa9d730c08aefefe3671d073d93d970bb39a31 Mon Sep 17 00:00:00 2001
From: fritsch 
Date: Sun, 29 Nov 2015 16:38:14 +0100
Subject: [PATCH] Intel: Implement Video Color Range

---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c |  4 ++--
 drivers/gpu/drm/i915/intel_drv.h |  8 
 drivers/gpu/drm/i915/intel_hdmi.c| 17 +++--
 drivers/gpu/drm/i915/intel_modes.c   |  1 +
 5 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 95bb27d..e453461 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3449,6 +3449,7 @@ int intel_freq_opcode(struct drm_i915_private
*dev_priv, int val);
 #define INTEL_BROADCAST_RGB_AUTO 0
 #define INTEL_BROADCAST_RGB_FULL 1
 #define INTEL_BROADCAST_RGB_LIMITED 2
+#define INTEL_BROADCAST_RGB_VIDEO 3

 static inline uint32_t i915_vgacntrl_reg(struct drm_device *dev)
 {
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 71860f8..ea40d81 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8605,7 +8605,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
   * consideration.
   */

- if (intel_crtc->config->limited_color_range)
+ if (intel_crtc->config->limited_color_range &&
!intel_crtc->config->video_color_range)
  coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */

  /*
@@ -8629,7 +8629,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
  if (INTEL_INFO(dev)->gen > 6) {
  uint16_t postoff = 0;

- if (intel_crtc->config->limited_color_range)
+ if (intel_crtc->config->limited_color_range &&
!intel_crtc->config->video_color_range)
  postoff = (16 * (1 << 12) / 255) & 0x1fff;

  I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 0598932..6940243 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -372,6 +372,13 @@ struct intel_crtc_state {
   */
  bool limited_color_range;

+ /*
+  *
+  * Use reduced/limited/broadcast rgb range without compressing.
+  *
+  */
+ bool video_color_range;
+
  /* DP has a bunch of special case unfortunately, so mark the pipe
   * accordingly. */
  bool has_dp_encoder;
@@ -682,6 +689,7 @@ struct intel_hdmi {
  int ddc_bus;
  bool limited_color_range;
  bool color_range_auto;
+ bool color_range_video;
  bool has_hdmi_sink;
  bool has_audio;
  enum hdmi_force_audio force_audio;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 9eafa19..dc78760 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -464,7 +464,8 @@ static void intel_hdmi_set_avi_infoframe(struct
drm_encoder *encoder,
  }

  if (intel_hdmi->rgb_quant_range_selectable) {
- if (intel_crtc->config->limited_color_range)
+ if (intel_crtc->config->limited_color_range ||
+ intel_crtc->config->video_color_range)