Re: [RESEND-CI v4 11/15] drm/i915: prepare scaler for YCBCR420 modeset

[Ander Conselvan De Oliveira]

On Fri, 2017-06-30 at 17:29 +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 6/30/2017 5:04 PM, Ander Conselvan De Oliveira wrote:
> > On Fri, 2017-06-30 at 11:20 +0530, Sharma, Shashank wrote:
> > > Regards
> > > 
> > > Shashank
> > > 
> > > 
> > > On 6/27/2017 5:46 PM, Ander Conselvan De Oliveira wrote:
> > > > On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> > > > > To get a YCBCR420 output from intel platforms, we need one
> > > > > scaler to scale down YCBCR444 samples to YCBCR420 samples.
> > > > > 
> > > > > This patch:
> > > > > - Does scaler allocation for HDMI ycbcr420 outputs.
> > > > > - Programs PIPE_MISC register for ycbcr420 output.
> > > > > - Adds a new scaler user "HDMI output" to plug-into existing
> > > > > scaler framework. This output type is identified using bit
> > > > > 30 of the scaler users bitmap.
> > > > > 
> > > > > V2: rebase
> > > > > V3: rebase
> > > > > V4: rebase
> > > > > 
> > > > > Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> > > > > Cc: Ander Conselvan De Oliveira 
> > > > > <ander.conselvan.de.olive...@intel.com>
> > > > > Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> > > > > ---
> > > > >drivers/gpu/drm/i915/intel_atomic.c  |  6 ++
> > > > >drivers/gpu/drm/i915/intel_display.c | 26 
> > > > > ++
> > > > >drivers/gpu/drm/i915/intel_drv.h | 10 +-
> > > > >drivers/gpu/drm/i915/intel_hdmi.c| 10 ++
> > > > >drivers/gpu/drm/i915/intel_panel.c   |  3 ++-
> > > > >5 files changed, 53 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> > > > > b/drivers/gpu/drm/i915/intel_atomic.c
> > > > > index 36d4e63..a8c9ac5 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > > > > @@ -264,6 +264,12 @@ int intel_atomic_setup_scalers(struct 
> > > > > drm_i915_private *dev_priv,
> > > > >
> > > > >   /* panel fitter case: assign as a crtc scaler */
> > > > >   scaler_id = _state->scaler_id;
> > > > > + } else if (i == SKL_HDMI_OUTPUT_INDEX) {
> > > > > + name = "HDMI-OUTPUT";
> > > > > + idx = intel_crtc->base.base.id;
> > > > > +
> > > > > + /* hdmi output case: needs a pipe scaler */
> > > > > + scaler_id = _state->scaler_id;
> > > > >   } else {
> > > > >   name = "PLANE";
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > index da29536..983f581 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state 
> > > > > *crtc_state, bool force_detach,
> > > > >*/
> > > > >   need_scaling = src_w != dst_w || src_h != dst_h;
> > > > >
> > > > > + if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420
> > > > > + && scaler_user == SKL_HDMI_OUTPUT_INDEX)
> > > > 
> > > > Is it really necessary to check for both? If it is, what's the point of 
> > > > creating
> > > > SKL_HDMI_OUTPUT_INDEX?
> > > 
> > > Yes, else this will affect scaler update for planes, as this function
> > > gets called to update the plane scalers too, at that time the output
> > > will be still valid (as its at CRTC level), but the
> > > scaler user would be different.
> > 
> > Right, so there is a need to check for scaler_user == HDMI_OUTPUT. But then
> > wouldn't it be a bug if hdmi_output != YCBCR420 ?  Point is, if the caller 
> > asked
> > for a HDMI_OUTPUT scaler hopefully it knows what its doing, no need to 
> > second
> >

Re: [Intel-gfx] [RESEND-CI v4 15/15] drm/i915/glk: set HDMI 2.0 identifier

[Ander Conselvan De Oliveira]

On Fri, 2017-06-30 at 17:47 +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 6/30/2017 5:37 PM, Ander Conselvan De Oliveira wrote:
> > On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> > > This patch sets the is_hdmi2_src identifier in drm connector
> > > for GLK platform. GLK contains a native HDMI 2.0 controller.
> > > This identifier will help the EDID handling functions to save
> > > lot of work which is specific to HDMI 2.0 sources.
> > > 
> > > V3: Added this patch
> > > V4: Rebase
> > > 
> > > Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> > 
> > This and patches 12 and 14 look fine to me. I'm not sure about the patch 
> > split,
> > maybe they should be squashed together in the end? And perhaps patch 10 and 
> > 13
> > too if the refactor I proposed are separate prep patches.
> 
> In fact this is exactly how I prepared at the first place, keeping all 
> the crtc/pipe level changes together.
> But then I thought the patch might be touching too many things, and 
> might be too big or complex for
> review, that's why I had split into 3-4 small patches :-)
> >   But anyway, you can
> > use
> > 
> > Reviewed-by: Ander Conselvan de Oliveira <conselv...@gmail.com>
> > 
> > on those if you want.
> 
> Thanks, I guess this applies for 12,14,10,13 and 15 (with a separate 
> patch for CSC coeff handling).
> Please correct me if I misunderstood.

This applies to 12, 14 and 15.

Ander

> 
> - Shashank
> > 
> > 
> > > ---
> > >   drivers/gpu/drm/i915/intel_hdmi.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> > > b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index 3bd9af3..0d9d088 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -1979,6 +1979,9 @@ void intel_hdmi_init_connector(struct 
> > > intel_digital_port *intel_dig_port,
> > >   connector->doublescan_allowed = 0;
> > >   connector->stereo_allowed = 1;
> > >   
> > > + if (IS_GEMINILAKE(dev_priv))
> > > + connector->ycbcr_420_allowed = true;
> > > +
> > >   intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > >   
> > >   switch (port) {
> 
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [RESEND-CI v4 15/15] drm/i915/glk: set HDMI 2.0 identifier

[Ander Conselvan De Oliveira]

On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> This patch sets the is_hdmi2_src identifier in drm connector
> for GLK platform. GLK contains a native HDMI 2.0 controller.
> This identifier will help the EDID handling functions to save
> lot of work which is specific to HDMI 2.0 sources.
> 
> V3: Added this patch
> V4: Rebase
> 
> Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>

This and patches 12 and 14 look fine to me. I'm not sure about the patch split,
maybe they should be squashed together in the end? And perhaps patch 10 and 13
too if the refactor I proposed are separate prep patches. But anyway, you can
use

Reviewed-by: Ander Conselvan de Oliveira <conselv...@gmail.com>

on those if you want.


> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 3bd9af3..0d9d088 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1979,6 +1979,9 @@ void intel_hdmi_init_connector(struct 
> intel_digital_port *intel_dig_port,
>   connector->doublescan_allowed = 0;
>   connector->stereo_allowed = 1;
>  
> + if (IS_GEMINILAKE(dev_priv))
> + connector->ycbcr_420_allowed = true;
> +
>   intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
>  
>   switch (port) {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RESEND-CI v4 11/15] drm/i915: prepare scaler for YCBCR420 modeset

[Ander Conselvan De Oliveira]

On Fri, 2017-06-30 at 11:20 +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 6/27/2017 5:46 PM, Ander Conselvan De Oliveira wrote:
> > On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> > > To get a YCBCR420 output from intel platforms, we need one
> > > scaler to scale down YCBCR444 samples to YCBCR420 samples.
> > > 
> > > This patch:
> > > - Does scaler allocation for HDMI ycbcr420 outputs.
> > > - Programs PIPE_MISC register for ycbcr420 output.
> > > - Adds a new scaler user "HDMI output" to plug-into existing
> > >scaler framework. This output type is identified using bit
> > >30 of the scaler users bitmap.
> > > 
> > > V2: rebase
> > > V3: rebase
> > > V4: rebase
> > > 
> > > Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> > > Cc: Ander Conselvan De Oliveira <ander.conselvan.de.olive...@intel.com>
> > > Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_atomic.c  |  6 ++
> > >   drivers/gpu/drm/i915/intel_display.c | 26 ++
> > >   drivers/gpu/drm/i915/intel_drv.h | 10 +-
> > >   drivers/gpu/drm/i915/intel_hdmi.c| 10 ++
> > >   drivers/gpu/drm/i915/intel_panel.c   |  3 ++-
> > >   5 files changed, 53 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> > > b/drivers/gpu/drm/i915/intel_atomic.c
> > > index 36d4e63..a8c9ac5 100644
> > > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > > @@ -264,6 +264,12 @@ int intel_atomic_setup_scalers(struct 
> > > drm_i915_private *dev_priv,
> > >   
> > >   /* panel fitter case: assign as a crtc scaler */
> > >   scaler_id = _state->scaler_id;
> > > + } else if (i == SKL_HDMI_OUTPUT_INDEX) {
> > > + name = "HDMI-OUTPUT";
> > > + idx = intel_crtc->base.base.id;
> > > +
> > > + /* hdmi output case: needs a pipe scaler */
> > > + scaler_id = _state->scaler_id;
> > >   } else {
> > >   name = "PLANE";
> > >   
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index da29536..983f581 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state 
> > > *crtc_state, bool force_detach,
> > >*/
> > >   need_scaling = src_w != dst_w || src_h != dst_h;
> > >   
> > > + if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420
> > > + && scaler_user == SKL_HDMI_OUTPUT_INDEX)
> > 
> > Is it really necessary to check for both? If it is, what's the point of 
> > creating
> > SKL_HDMI_OUTPUT_INDEX?
> 
> Yes, else this will affect scaler update for planes, as this function 
> gets called to update the plane scalers too, at that time the output 
> will be still valid (as its at CRTC level), but the
> scaler user would be different.

Right, so there is a need to check for scaler_user == HDMI_OUTPUT. But then
wouldn't it be a bug if hdmi_output != YCBCR420 ?  Point is, if the caller asked
for a HDMI_OUTPUT scaler hopefully it knows what its doing, no need to second
guess it.

> > 
> > > + /* YCBCR 444 -> 420 conversion needs a scaler */
> > > + need_scaling = true;
> > > +
> > >   /*
> > >* if plane is being disabled or scaler is no more required or 
> > > force detach
> > >*  - free scaler binded to this plane/crtc
> > > @@ -4673,6 +4678,26 @@ skl_update_scaler(struct intel_crtc_state 
> > > *crtc_state, bool force_detach,
> > >   }
> > >   
> > >   /**
> > > + * skl_update_scaler_hdmi_output - Stages update to scaler state for 
> > > HDMI.
> > > + * HDMI YCBCR 420 output needs a scaler, for downsampling.
> > > + *
> > > + * @state: crtc's scaler state
> > > + *
> > > + * Return
> > > + * 0 - scaler_usage updated successfully
> > > + *error - requested scaling cannot be supported or other error 
> > > condition
> > >

Re: [RESEND-CI v4 13/15] drm/i915: prepare csc unit for YCBCR HDMI output

[Ander Conselvan De Oliveira]

On Fri, 2017-06-30 at 11:33 +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 6/29/2017 5:38 PM, Ander Conselvan De Oliveira wrote:
> > On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> > > To support ycbcr HDMI output, we need a pipe CSC block to
> > > do the RGB->YCBCR conversion, as the blender output is in RGB.
> > > 
> > > Current Intel platforms have only one pipe CSC unit, so
> > > we can either do color correction using it, or we can perform
> > > RGB->YCBCR conversion.
> > > 
> > > This function adds a csc handler, to perform RGB->YCBCR conversion
> > > as per recommended spec values.
> > 
> > Please do a full reference to the "spec", including name, version and 
> > relevant
> > section.
> 
> Sure, will add more details.
> > > V2: Rebase
> > > V3: Rebase
> > > V4: Rebase
> > > 
> > > Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vet...@intel.com>
> > > Cc: Ander Conselvan De Oliveira <ander.conselvan.de.olive...@intel.com>
> > > Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_color.c   | 47 
> > > +++-
> > >   drivers/gpu/drm/i915/intel_display.c | 32 
> > >   2 files changed, 78 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_color.c 
> > > b/drivers/gpu/drm/i915/intel_color.c
> > > index 306c6b0..12d5f21 100644
> > > --- a/drivers/gpu/drm/i915/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/intel_color.c
> > > @@ -41,6 +41,19 @@
> > >   
> > >   #define LEGACY_LUT_LENGTH   (sizeof(struct drm_color_lut) * 
> > > 256)
> > >   
> > > +/* Post offset values for RGB->YCBCR conversion */
> > > +#define POSTOFF_RGB_TO_YUV_HI 0x800
> > > +#define POSTOFF_RGB_TO_YUV_ME 0x100
> > > +#define POSTOFF_RGB_TO_YUV_LO 0x800
> > > +
> > > +/* Direct spec values for RGB->YUV conversion matrix */
> > > +#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8
> > > +#define CSC_RGB_TO_YUV_BU 0x37e8
> > > +#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0
> > > +#define CSC_RGB_TO_YUV_BY 0xb528
> > > +#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8
> > > +#define CSC_RGB_TO_YUV_BV 0x1e08
> > > +
> > 
> > Not a big fan or hardcoding this in the register format. We already have the
> > code for converting a number to the right format for the register in
> > i915_load_csc_matrix(). I think it would make more sense to extract the code
> > that actually writes the matrix out of that function, so it would just
> > unconditionally use a matrix and coefficients passed as arguments. Then the
> > values above would be defined in the format expected for this new function.
> 
> Actually I had a small discussion on this with Ville, and we think that 
> the current CSC multiplication code is not correct.
> So if CTM and limited color range is applied together, we might not be 
> getting the right values. So not planning to
> reuse that code. I think while sending the BT2020 patches, we will add a 
> fix for that part too, but right now not working on it.

I wasn't talking about the matrix multiplication, but creating a function to
write any given matrix into the CSC. That way, the above could be hardcoded in a
more human readable format.

This issue is independent from fixing the matrix multiplication. I'm talking
specifically about the code below:

/*
 * Convert fixed point S31.32 input to format supported by the
 * hardware.
 */
for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
uint64_t abs_coeff = ((1ULL << 63) - 1) & input[i];

/*
 * Clamp input value to min/max supported by
 * hardware.
 */
abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1);

/* sign bit */
if (CTM_COEFF_NEGATIVE(input[i]))
coeffs[i] |= 1 << 15;

if (abs_coeff < CTM_COEFF_0_125)
coeffs[i] |= (3 << 12) |
I9XX_CSC_COEFF_FP(abs_coeff, 12);
else if (abs_coeff < CTM_COEFF_0_25)
coeffs[i] |= (2 << 12) |

Re: [RESEND-CI v4 13/15] drm/i915: prepare csc unit for YCBCR HDMI output

[Ander Conselvan De Oliveira]

On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> To support ycbcr HDMI output, we need a pipe CSC block to
> do the RGB->YCBCR conversion, as the blender output is in RGB.
> 
> Current Intel platforms have only one pipe CSC unit, so
> we can either do color correction using it, or we can perform
> RGB->YCBCR conversion.
> 
> This function adds a csc handler, to perform RGB->YCBCR conversion
> as per recommended spec values.

Please do a full reference to the "spec", including name, version and relevant
section.

> 
> V2: Rebase
> V3: Rebase
> V4: Rebase
> 
> Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Ander Conselvan De Oliveira <ander.conselvan.de.olive...@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 47 
> +++-
>  drivers/gpu/drm/i915/intel_display.c | 32 
>  2 files changed, 78 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c 
> b/drivers/gpu/drm/i915/intel_color.c
> index 306c6b0..12d5f21 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -41,6 +41,19 @@
>  
>  #define LEGACY_LUT_LENGTH(sizeof(struct drm_color_lut) * 256)
>  
> +/* Post offset values for RGB->YCBCR conversion */
> +#define POSTOFF_RGB_TO_YUV_HI 0x800
> +#define POSTOFF_RGB_TO_YUV_ME 0x100
> +#define POSTOFF_RGB_TO_YUV_LO 0x800
> +
> +/* Direct spec values for RGB->YUV conversion matrix */
> +#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8
> +#define CSC_RGB_TO_YUV_BU 0x37e8
> +#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0
> +#define CSC_RGB_TO_YUV_BY 0xb528
> +#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8
> +#define CSC_RGB_TO_YUV_BV 0x1e08
> +

Not a big fan or hardcoding this in the register format. We already have the
code for converting a number to the right format for the register in
i915_load_csc_matrix(). I think it would make more sense to extract the code
that actually writes the matrix out of that function, so it would just
unconditionally use a matrix and coefficients passed as arguments. Then the
values above would be defined in the format expected for this new function.

>  /*
>   * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>   * format). This macro takes the coefficient we want transformed and the
> @@ -91,6 +104,35 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t 
> *input)
>   }
>  }
>  
> +void i9xx_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc)
> +{
> + int pipe = intel_crtc->pipe;
> + struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +
> + /* We don't use high values for conversion */
> + I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
> + I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
> + I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0);
> +
> + /* Program direct spec values for RGB to YCBCR conversion matrix */
> + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), CSC_RGB_TO_YUV_RU_GU);
> + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), CSC_RGB_TO_YUV_BU);
> +
> + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), CSC_RGB_TO_YUV_RY_GY);
> + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), CSC_RGB_TO_YUV_BY);
> +
> + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), CSC_RGB_TO_YUV_RV_GV);
> + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), CSC_RGB_TO_YUV_BV);
> +
> + /* Spec postoffset values */
> + I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI);
> + I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME);
> + I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO);
> +
> + /* CSC mode before gamma */
> + I915_WRITE(PIPE_CSC_MODE(pipe), 0);
> +}
> +
>  /* Set up the pipe CSC unit. */
>  static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
>  {
> @@ -101,7 +143,10 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state 
> *crtc_state)
>   uint16_t coeffs[9] = { 0, };
>   struct intel_crtc_state *intel_crtc_state = 
> to_intel_crtc_state(crtc_state);
>  
> - if (crtc_state->ctm) {
> + if (intel_crtc_state->hdmi_output > DRM_HDMI_OUTPUT_DEFAULT_RGB) {
> + i9xx_load_ycbcr_conversion_matrix(intel_crtc);
> + return;
> + } else if (crtc_state->ctm) {

Hmm, I'm not sure this is the right place to check for this condition. I mean,
we shouldn't allow the modeset to happen in this case.

>   struct drm_color_ctm *ctm =
>   (struct drm_color_ctm *)crtc_state->ctm->d

Re: [RESEND-CI v4 11/15] drm/i915: prepare scaler for YCBCR420 modeset

[Ander Conselvan De Oliveira]

On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> To get a YCBCR420 output from intel platforms, we need one
> scaler to scale down YCBCR444 samples to YCBCR420 samples.
> 
> This patch:
> - Does scaler allocation for HDMI ycbcr420 outputs.
> - Programs PIPE_MISC register for ycbcr420 output.
> - Adds a new scaler user "HDMI output" to plug-into existing
>   scaler framework. This output type is identified using bit
>   30 of the scaler users bitmap.
> 
> V2: rebase
> V3: rebase
> V4: rebase
> 
> Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> Cc: Ander Conselvan De Oliveira <ander.conselvan.de.olive...@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |  6 ++
>  drivers/gpu/drm/i915/intel_display.c | 26 ++
>  drivers/gpu/drm/i915/intel_drv.h | 10 +-
>  drivers/gpu/drm/i915/intel_hdmi.c| 10 ++
>  drivers/gpu/drm/i915/intel_panel.c   |  3 ++-
>  5 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 36d4e63..a8c9ac5 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -264,6 +264,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private 
> *dev_priv,
>  
>   /* panel fitter case: assign as a crtc scaler */
>   scaler_id = _state->scaler_id;
> + } else if (i == SKL_HDMI_OUTPUT_INDEX) {
> + name = "HDMI-OUTPUT";
> + idx = intel_crtc->base.base.id;
> +
> + /* hdmi output case: needs a pipe scaler */
> + scaler_id = _state->scaler_id;
>   } else {
>   name = "PLANE";
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index da29536..983f581 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, 
> bool force_detach,
>*/
>   need_scaling = src_w != dst_w || src_h != dst_h;
>  
> + if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420
> + && scaler_user == SKL_HDMI_OUTPUT_INDEX)

Is it really necessary to check for both? If it is, what's the point of creating
SKL_HDMI_OUTPUT_INDEX?


> + /* YCBCR 444 -> 420 conversion needs a scaler */
> + need_scaling = true;
> +
>   /*
>* if plane is being disabled or scaler is no more required or force 
> detach
>*  - free scaler binded to this plane/crtc
> @@ -4673,6 +4678,26 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, 
> bool force_detach,
>  }
>  
>  /**
> + * skl_update_scaler_hdmi_output - Stages update to scaler state for HDMI.
> + * HDMI YCBCR 420 output needs a scaler, for downsampling.
> + *
> + * @state: crtc's scaler state
> + *
> + * Return
> + * 0 - scaler_usage updated successfully
> + *error - requested scaling cannot be supported or other error condition
> + */
> +int skl_update_scaler_crtc_hdmi_output(struct intel_crtc_state *state)
> +{
> + const struct drm_display_mode *mode = >base.adjusted_mode;
> +
> + return skl_update_scaler(state, !state->base.active,
> + SKL_HDMI_OUTPUT_INDEX, >scaler_state.scaler_id,
> + state->pipe_src_w, state->pipe_src_h,
> + mode->crtc_hdisplay, mode->crtc_vdisplay);
> +}
> +
> +/**
>   * skl_update_scaler_crtc - Stages update to scaler state for a given crtc.
>   *
>   * @state: crtc's scaler state
> @@ -8058,6 +8083,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
>  {
>   struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + enum drm_hdmi_output_type hdmi_out = intel_crtc->config->hdmi_output;
>  
>   if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) {
>   u32 val = 0;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 38fe56c..2206aa8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -471,7 +471,8 @@ struct intel_crtc_scaler_state {
>*
>* If a bit is set, a user is using a scaler.
>* Here user can be a plane or crtc as defined below:
> -  *   bits 0-30 - plane (bit position is index from drm_plane_index)
> +

Re: [Intel-gfx] [PATCH v4 10/15] drm/i915: add compute-config for YCBCR outputs

[Ander Conselvan De Oliveira]

On Wed, 2017-06-21 at 21:19 +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 6/20/2017 7:50 PM, Ander Conselvan De Oliveira wrote:
> > On Mon, 2017-06-19 at 21:38 +0530, Shashank Sharma wrote:
> > > This patch checks encoder level support for HDMI YCBCR outputs.
> > > HDMI output mode is a connector property, this patch checks if
> > > source and sink can support the HDMI output type selected by user.
> > > And if they both can, it commits the hdmi output type into crtc state,
> > > for further staging in driver.
> > > 
> > > V2: Split the patch into two, kept helper functions in DRM layer.
> > > V3: Changed the compute_config function based on new DRM API.
> > > V4: Rebase
> > > 
> > > Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vet...@intel.com>
> > > Cc: Ander Conselvan De Oliveira <ander.conselvan.de.olive...@intel.com>
> > > Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_display.c |  1 +
> > >   drivers/gpu/drm/i915/intel_drv.h |  3 ++
> > >   drivers/gpu/drm/i915/intel_hdmi.c| 93 
> > > ++--
> > >   3 files changed, 94 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index f9bf0d5..da29536 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11930,6 +11930,7 @@ intel_pipe_config_compare(struct drm_i915_private 
> > > *dev_priv,
> > >   PIPE_CONF_CHECK_I(hdmi_scrambling);
> > >   PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
> > >   PIPE_CONF_CHECK_I(has_infoframe);
> > > + PIPE_CONF_CHECK_I(hdmi_output);
> > >   
> > >   PIPE_CONF_CHECK_I(has_audio);
> > >   
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 1727350..38fe56c 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -780,6 +780,9 @@ struct intel_crtc_state {
> > >   
> > >   /* HDMI High TMDS char rate ratio */
> > >   bool hdmi_high_tmds_clock_ratio;
> > > +
> > > + /* HDMI output type */
> > > + enum drm_hdmi_output_type hdmi_output;
> > >   };
> > >   
> > >   struct intel_crtc {
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> > > b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index 170abc4..22da5cd 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -1317,7 +1317,8 @@ intel_hdmi_mode_valid(struct drm_connector 
> > > *connector,
> > >   return status;
> > >   }
> > >   
> > > -static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
> > > +static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state,
> > > + enum drm_hdmi_output_type hdmi_out)
> > >   {
> > >   struct drm_i915_private *dev_priv =
> > >   to_i915(crtc_state->base.crtc->dev);
> > > @@ -1329,6 +1330,16 @@ static bool hdmi_12bpc_possible(struct 
> > > intel_crtc_state *crtc_state)
> > >   if (HAS_GMCH_DISPLAY(dev_priv))
> > >   return false;
> > >   
> > > + if (hdmi_out == DRM_HDMI_OUTPUT_YCBCR422) {
> > > + /*
> > > +  * HDMI spec says YCBCR422 is 12bpc, but its not a deep
> > > +  * color format. So respect the spec, and not allow this
> > > +  * to be deep color
> > > +  */
> > > + DRM_DEBUG_KMS("Not allowing deep color for YCBCR422 output\n");
> > > + return false;
> > > + }
> > > +
> > 
> > I suppose this is about the remark in section 6.2.4 of the HDMI 1.4 spec. 
> > If I'm
> > interpreting that correctly, the relevant information for us is that the 
> > TMDS
> > clock doesn't need to be multiplied by 1.5, unlike other 12 bpc scenarios.
> > 
> > To me it seems misleading to say 12 bpc is not possible in that case. I'm
> > wondering if it makes more sense to split this in two parts:
> > 
> >   (i)  What is the TMDS clock scaling for a given output f

Re: [Intel-gfx] [PATCH v4 10/15] drm/i915: add compute-config for YCBCR outputs

[Ander Conselvan De Oliveira]

On Mon, 2017-06-19 at 21:38 +0530, Shashank Sharma wrote:
> This patch checks encoder level support for HDMI YCBCR outputs.
> HDMI output mode is a connector property, this patch checks if
> source and sink can support the HDMI output type selected by user.
> And if they both can, it commits the hdmi output type into crtc state,
> for further staging in driver.
> 
> V2: Split the patch into two, kept helper functions in DRM layer.
> V3: Changed the compute_config function based on new DRM API.
> V4: Rebase
> 
> Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Ander Conselvan De Oliveira <ander.conselvan.de.olive...@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  1 +
>  drivers/gpu/drm/i915/intel_drv.h |  3 ++
>  drivers/gpu/drm/i915/intel_hdmi.c| 93 
> ++--
>  3 files changed, 94 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index f9bf0d5..da29536 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11930,6 +11930,7 @@ intel_pipe_config_compare(struct drm_i915_private 
> *dev_priv,
>   PIPE_CONF_CHECK_I(hdmi_scrambling);
>   PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
>   PIPE_CONF_CHECK_I(has_infoframe);
> + PIPE_CONF_CHECK_I(hdmi_output);
>  
>   PIPE_CONF_CHECK_I(has_audio);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1727350..38fe56c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -780,6 +780,9 @@ struct intel_crtc_state {
>  
>   /* HDMI High TMDS char rate ratio */
>   bool hdmi_high_tmds_clock_ratio;
> +
> + /* HDMI output type */
> + enum drm_hdmi_output_type hdmi_output;
>  };
>  
>  struct intel_crtc {
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 170abc4..22da5cd 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1317,7 +1317,8 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>   return status;
>  }
>  
> -static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
> +static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state,
> + enum drm_hdmi_output_type hdmi_out)
>  {
>   struct drm_i915_private *dev_priv =
>   to_i915(crtc_state->base.crtc->dev);
> @@ -1329,6 +1330,16 @@ static bool hdmi_12bpc_possible(struct 
> intel_crtc_state *crtc_state)
>   if (HAS_GMCH_DISPLAY(dev_priv))
>   return false;
>  
> + if (hdmi_out == DRM_HDMI_OUTPUT_YCBCR422) {
> + /*
> +  * HDMI spec says YCBCR422 is 12bpc, but its not a deep
> +  * color format. So respect the spec, and not allow this
> +  * to be deep color
> +  */
> + DRM_DEBUG_KMS("Not allowing deep color for YCBCR422 output\n");
> + return false;
> + }
> +

I suppose this is about the remark in section 6.2.4 of the HDMI 1.4 spec. If I'm
interpreting that correctly, the relevant information for us is that the TMDS
clock doesn't need to be multiplied by 1.5, unlike other 12 bpc scenarios.

To me it seems misleading to say 12 bpc is not possible in that case. I'm
wondering if it makes more sense to split this in two parts:

 (i)  What is the TMDS clock scaling for a given output format?
 (ii) Is the format and scaled TMDS clock supported by the source and the sink?


>   /*
>* HDMI 12bpc affects the clocks, so it's only possible
>* when not cloning with other encoder types.
> @@ -1342,6 +1353,12 @@ static bool hdmi_12bpc_possible(struct 
> intel_crtc_state *crtc_state)
>   if (connector_state->crtc != crtc_state->base.crtc)
>   continue;
>  
> + if (hdmi_out == DRM_HDMI_OUTPUT_YCBCR420) {
> + if (!(info->hdmi.y420_dc_modes &
> + DRM_EDID_YCBCR420_DC_36))
> + return false;
> + }
> +
>   if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0)
>   return false;
>   }
> @@ -1354,6 +1371,65 @@ static bool hdmi_12bpc_possible(struct 
> intel_crtc_state *crtc_state)
>   return true;
>  }
>  
> +static u8
> +intel_hdmi_get_src_output_support(struct drm_connector *connector)
> +{
> + 

Re: [Intel-gfx] [PATCH v3] drm: Add DPCD definitions for DP 1.4 DSC feature

[Ander Conselvan De Oliveira]

On Tue, 2017-03-14 at 13:01 -0700, Manasi Navare wrote:
> From: "Navare, Manasi D" 
> 
> Display stream compression is supported on DP 1.4 DP
> devices. This patch adds the corersponding DPCD
> register definitions for DSC.
> 
> v3:
> * Add some SHIFTS and MASKS for uniformity (Jani Nikula)
> v2:
> * Rebased on drm-tip
> 
> Signed-off-by: Manasi Navare 
> Cc: Jani Nikula 
> Cc: Paulo Zanoni 
> Cc: dri-devel@lists.freedesktop.org
> ---
>  include/drm/drm_dp_helper.h | 105 
> 
>  1 file changed, 105 insertions(+)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index c0bd0d7..e1fb04f 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -179,6 +179,111 @@
>  
>  #define DP_GUID  0x030   /* 1.2 */
>  
> +#define DP_DSC_SUPPORT  0x060   /* DP 1.4 */
> +# define DP_DSC_DECOMPRESSION_IS_SUPPORTED  (1 << 0)
> +
> +#define DP_DSC_REV  0x061
> +# define DP_DSC_MAJOR_MASK  (0xf << 0)
> +# define DP_DSC_MINOR_MASK  (0xf << 4)
> +# define DP_DSC_MAJOR_SHIFT 0
> +# define DP_DSC_MINOR_SHIFT 4
> +
> +#define DP_DSC_RC_BUF_BLK_SIZE  0x062
> +# define DP_DSC_RC_BUF_BLK_SIZE_1   0x0
> +# define DP_DSC_RC_BUF_BLK_SIZE_4   0x1
> +# define DP_DSC_RC_BUF_BLK_SIZE_16  0x2
> +# define DP_DSC_RC_BUF_BLK_SIZE_64  0x3
> +
> +#define DP_DSC_RC_BUF_SIZE  0x063
> +
> +#define DP_DSC_SLICE_CAP_1  0x064
> +# define DP_DSC_1_PER_DP_DSC_SINK   (1 << 0)
> +# define DP_DSC_2_PER_DP_DSC_SINK   (1 << 1)
> +# define DP_DSC_4_PER_DP_DSC_SINK   (1 << 3)
> +# define DP_DSC_6_PER_DP_DSC_SINK   (1 << 4)
> +# define DP_DSC_8_PER_DP_DSC_SINK   (1 << 5)
> +# define DP_DSC_10_PER_DP_DSC_SINK  (1 << 6)
> +# define DP_DSC_12_PER_DP_DSC_SINK  (1 << 7)
> +
> +#define DP_DSC_LINE_BUF_BIT_DEPTH   0x065
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_MASK (0xf << 0)
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_90x0
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_10   0x1
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_11   0x2
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_12   0x3
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_13   0x4
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_14   0x5
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_15   0x6
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_16   0x7
> +# define DP_DSC_LINE_BUF_BIT_DEPTH_80x8
> +
> +#define DP_DSC_BLK_PREDICTION_SUPPORT   0x066
> +# define DP_DSC_BLK_PREDICTION_IS_SUPPORTED (1 << 0)
> +
> +#define DP_DSC_MAX_BITS_PER_PIXEL_LOW   0x067   /* eDP 1.4 */
> +
> +#define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068   /* eDP 1.4 */
> +
> +#define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069
> +# define DP_DSC_RGB (1 << 0)
> +# define DP_DSC_YCbCr444(1 << 1)
> +# define DP_DSC_YCbCr422_Simple (1 << 2)
> +# define DP_DSC_YCbCr422_Native (1 << 3)
> +# define DP_DSC_YCbCr420_Native (1 << 4)
> +
> +#define DP_DSC_DEC_COLOR_DEPTH_CAP  0x06A
> +# define DP_DSC_8_BPC   (1 << 1)
> +# define DP_DSC_10_BPC  (1 << 2)
> +# define DP_DSC_12_BPC  (1 << 3)
> +
> +#define DP_DSC_PEAK_THROUGHPUT  0x06B
> +# define DP_DSC_THROUGHPUT_MODE_0_MASK  (0xf << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_SHIFT 0
> +# define DP_DSC_THROUGHPUT_MODE_0_340   (1 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_400   (2 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_450   (3 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_500   (4 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_550   (5 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_600   (6 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_650   (7 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_700   (8 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_750   (9 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_800   (10 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_850   (11 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_900   (12 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_950   (13 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_0_1000  (14 << 0)
> +# define DP_DSC_THROUGHPUT_MODE_1_MASK  (0xf << 4)
> +# define DP_DSC_THROUGHPUT_MODE_1_SHIFT 4
> +# define DP_DSC_THROUGHPUT_MODE_1_340   (1 << 4)
> +# define DP_DSC_THROUGHPUT_MODE_1_400   (2 << 4)
> +# define DP_DSC_THROUGHPUT_MODE_1_450   (3 << 4)
> +# define DP_DSC_THROUGHPUT_MODE_1_500   (4 << 4)
> +# define DP_DSC_THROUGHPUT_MODE_1_550   (5 << 4)
> +# define DP_DSC_THROUGHPUT_MODE_1_600   (6 << 4)
> +# define DP_DSC_THROUGHPUT_MODE_1_650   (7 << 4)
> +# define 

Re: [PATCH v6 1/3] drm_fourcc: Add new P010, P016 video format

[Ander Conselvan De Oliveira]

On Tue, 2017-03-07 at 04:27 +0800, Ayaka wrote:
> 
> 從我的 iPad 傳送
> 
> > Ville Syrjälä  於 2017年3月7日 上午2:34 寫道：
> > 
> > > On Tue, Mar 07, 2017 at 01:58:23AM +0800, Ayaka wrote:
> > > 
> > > 
> > > 從我的 iPad 傳送
> > > 
> > > > > Ville Syrjälä  於 2017年3月6日 下午9:06 寫道：
> > > > > 
> > > > > On Sun, Mar 05, 2017 at 06:00:31PM +0800, Randy Li wrote:
> > > > > P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits
> > > > > per channel video format.
> > > > > 
> > > > > P016 is a planar 4:2:0 YUV with interleaved UV plane, 16 bits
> > > > > per channel video format.
> > > > > 
> > > > > V3: Added P012 and fixed cpp for P010
> > > > > V4: format definition refined per review
> > > > > V5: Format comment block for each new pixel format
> > > > > V6: reversed Cb/Cr order in comments
> > > > > v7: reversed Cb/Cr order in comments of header files, remove
> > > > > the wrong part of commit message.
> > > > 
> > > > What? Why? You just undid what Clint did in v6.
> > > 
> > > He missed a file also keeping the wrong description of rockchip.
> > 
> > I don't follow. Who missed what exactly?
> 
> What he sent is v5, I increase the order number twice in the message, it 
> confuse me as well. 
> I think Clint forgot the include/uapi/drm/drm_fourcc.h .

Clint did send a v6, and that updates "include/uapi/drm/drm_fourcc.h":

https://patchwork.freedesktop.org/patch/141342/


Ander

> > 
> > 
> > > > 
> > > > > 
> > > > > Cc: Daniel Stone 
> > > > > Cc: Ville Syrjälä 
> > > > > 
> > > > > Signed-off-by: Randy Li 
> > > > > Signed-off-by: Clint Taylor 
> > > > > ---
> > > > > drivers/gpu/drm/drm_fourcc.c  |  3 +++
> > > > > include/uapi/drm/drm_fourcc.h | 21 +
> > > > > 2 files changed, 24 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_fourcc.c 
> > > > > b/drivers/gpu/drm/drm_fourcc.c
> > > > > index 90d2cc8..3e0fd58 100644
> > > > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > > > @@ -165,6 +165,9 @@ const struct drm_format_info 
> > > > > *__drm_format_info(u32 format)
> > > > >   { .format = DRM_FORMAT_UYVY,.depth = 0,  .num_planes = 
> > > > > 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> > > > >   { .format = DRM_FORMAT_VYUY,.depth = 0,  .num_planes = 
> > > > > 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> > > > >   { .format = DRM_FORMAT_AYUV,.depth = 0,  .num_planes = 
> > > > > 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> > > > > +{ .format = DRM_FORMAT_P010,.depth = 0,  .num_planes 
> > > > > = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
> > > > > +{ .format = DRM_FORMAT_P012,.depth = 0,  .num_planes 
> > > > > = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
> > > > > +{ .format = DRM_FORMAT_P016,.depth = 0,  .num_planes 
> > > > > = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
> > > > >   };
> > > > > 
> > > > >   unsigned int i;
> > > > > diff --git a/include/uapi/drm/drm_fourcc.h 
> > > > > b/include/uapi/drm/drm_fourcc.h
> > > > > index ef20abb..306f979 100644
> > > > > --- a/include/uapi/drm/drm_fourcc.h
> > > > > +++ b/include/uapi/drm/drm_fourcc.h
> > > > > @@ -128,6 +128,27 @@ extern "C" {
> > > > > #define DRM_FORMAT_NV42fourcc_code('N', 'V', '4', '2') /* 
> > > > > non-subsampled Cb:Cr plane */
> > > > > 
> > > > > /*
> > > > > + * 2 plane YCbCr MSB aligned
> > > > > + * index 0 = Y plane, [15:0] Y:x [10:6] little endian
> > > > > + * index 1 = Cb:Cr plane, [31:0] Cb:x:Cr:x [10:6:10:6] little endian
> > > > > + */
> > > > > +#define DRM_FORMAT_P010fourcc_code('P', '0', '1', '0') /* 
> > > > > 2x2 subsampled Cb:Cr plane 10 bits per channel */
> > > > > +
> > > > > +/*
> > > > > + * 2 plane YCbCr MSB aligned
> > > > > + * index 0 = Y plane, [15:0] Y:x [12:4] little endian
> > > > > + * index 1 = Cb:Cr plane, [31:0] Cb:x:Cr:x [12:4:12:4] little endian
> > > > > + */
> > > > > +#define DRM_FORMAT_P012fourcc_code('P', '0', '1', '2') /* 
> > > > > 2x2 subsampled Cb:Cr plane 12 bits per channel */
> > > > > +
> > > > > +/*
> > > > > + * 2 plane YCbCr MSB aligned
> > > > > + * index 0 = Y plane, [15:0] Y little endian
> > > > > + * index 1 = Cb:Cr plane, [31:0] Cb:Cr [16:16] little endian
> > > > > + */
> > > > > +#define DRM_FORMAT_P016fourcc_code('P', '0', '1', '6') /* 
> > > > > 2x2 subsampled Cb:Cr plane 16 bits per channel */
> > > > > +
> > > > > +/*
> > > > > * 3 plane YCbCr
> > > > > * index 0: Y plane, [7:0] Y
> > > > > * index 1: Cb plane, [7:0] Cb
> > > > > -- 
> > > > > 2.7.4
> > > > 
> > > > -- 
> > > > Ville Syrjälä
> > > > Intel OTC
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> 

Re: [Intel-gfx] [PATCH v10 5/6] drm/i915: enable scrambling

[Ander Conselvan De Oliveira]

On Mon, 2017-03-13 at 16:54 +0530, Shashank Sharma wrote:
> Geminilake platform sports a native HDMI 2.0 controller, and is
> capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
> mendates scrambling for these higher clocks, for reduced RF footprint.
> 
> This patch checks if the monitor supports scrambling, and if required,
> enables it during the modeset.
> 
> V2: Addressed review comments from Ville:
>  - Do not track scrambling status in DRM layer, track somewhere in
>driver like in intel_crtc_state.
>  - Don't talk to monitor at such a low layer, set monitor scrambling
>in intel_enable_ddi() before enabling the port.
> 
> V3: Addressed review comments from Jani
>  - In comments, function names, use "sink" instead of "monitor",
>so that the implementation could be close to the language of
>HDMI spec.
> 
> V4: Addressed review comment from Maarten
>  - scrambling -> hdmi_scrambling
>  - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> 
> V5: Addressed review comments from Ville and Ander
>  - Do not modifiy the crtc_state after compute_config. Move all
>scrambling and tmds_clock_ratio calcutations to compute_config.
>  - While setting scrambling for source/sink, do not check the
>conditions again, just go by the crtc_state flags. This will
>simplyfy the condition checks.
> 
> V6: Addressed review comments from Ville
>  - Do not add IS_GLK check in disable/enable function, instead add it
>in compute_config, while setting state flags.
>  - Remove unnecessary paranthesis.
>  - Simplyfy handle_sink_scrambling function as suggested.
>  - Add readout code for scrambling status in get_ddi_config and add a
>check for the same in pipe_config_compare.
> 
> V7: Addressed review comments from Ander/Ville
>  - No separate function for source scrambling, make it inline
>  - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
>  - Do not add platform check while setting source scrambling
>  - Use pipe_config instead of crtc->config to set sink scrambling
>  - To readout scrambling status, Compare with SCRAMBLING_MASK
>not any of its bits
>  - Remove platform check in intel_pipe_config_compare while checking
>scrambling status
> 
> V8: Fixed mege conflict, Addressed review comments from Ander
>  - Remove the desciption/comment about scrambling fom the caller, move
>it to the function
>  - Move the IS_GLK check into scrambling function
>  - Fix alignment
> 
> V9: Fixed review comments from Ville, Ander
>  - Pass the scrambling state variables as bool input to the sink_scrambling
>function and let the disable call be unconditional.
>  - Fix alignments in function calls and debug messages.
>  - Add kernel doc for function intel_hdmi_handle_sink_scrambling
> 
> V10: Rebase
> Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>

Reviewed-by: Ander Conselvan de Oliveira <conselv...@gmail.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  7 
>  drivers/gpu/drm/i915/intel_ddi.c | 23 +
>  drivers/gpu/drm/i915/intel_display.c |  3 ++
>  drivers/gpu/drm/i915/intel_drv.h | 10 ++
>  drivers/gpu/drm/i915/intel_hdmi.c| 63 
> 
>  5 files changed, 106 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 19d42e8..a1aae7a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7831,7 +7831,14 @@ enum {
>  #define  TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12)
>  #define  TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12)
>  #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC   (1<<8)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
>  #define  TRANS_DDI_BFI_ENABLE(1<<4)
> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE   (1<<4)
> +#define  TRANS_DDI_HDMI_SCRAMBLING   (1<<0)
> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE 
> \
> + | TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> + | TRANS_DDI_HDMI_SCRAMBLING)
>  
>  /* DisplayPort Transport Control */
>  #define _DP_TP_CTL_A 0x64040
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index ee341ef..169d2b4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct 
> intel_crtc_state *crtc_state)
>   temp |= TRANS_DDI_MODE_SELECT_HDMI;
>   

Re: [PATCH v8 5/6] drm/i915: enable scrambling

[Ander Conselvan De Oliveira]

On Wed, 2017-03-08 at 19:07 +0530, Shashank Sharma wrote:
>  Geminilake platform sports a native HDMI 2.0 controller, and is
>  capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
>  mendates scrambling for these higher clocks, for reduced RF footprint.
> 
>  This patch checks if the monitor supports scrambling, and if required,
>  enables it during the modeset.
> 
>  V2: Addressed review comments from Ville:
>  - Do not track scrambling status in DRM layer, track somewhere in
>driver like in intel_crtc_state.
>  - Don't talk to monitor at such a low layer, set monitor scrambling
>in intel_enable_ddi() before enabling the port.
> 
>  V3: Addressed review comments from Jani
>  - In comments, function names, use "sink" instead of "monitor",
>so that the implementation could be close to the language of
>HDMI spec.
> 
> V4: Addressed review comment from Maarten
>  - scrambling -> hdmi_scrambling
>  - high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> 
> V5: Addressed review comments from Ville and Ander
>  - Do not modifiy the crtc_state after compute_config. Move all
>scrambling and tmds_clock_ratio calcutations to compute_config.
>  - While setting scrambling for source/sink, do not check the
>conditions again, just go by the crtc_state flags. This will
>simplyfy the condition checks.
> 
> V6: Addressed review comments from Ville
>  - Do not add IS_GLK check in disable/enable function, instead add it
>in compute_config, while setting state flags.
>  - Remove unnecessary paranthesis.
>  - Simplyfy handle_sink_scrambling function as suggested.
>  - Add readout code for scrambling status in get_ddi_config and add a
>check for the same in pipe_config_compare.
> 
> V7: Addressed review comments from Ander/Ville
>  - No separate function for source scrambling, make it inline
>  - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
>  - Do not add platform check while setting source scrambling
>  - Use pipe_config instead of crtc->config to set sink scrambling
>  - To readout scrambling status, Compare with SCRAMBLING_MASK
>not any of its bits
>  - Remove platform check in intel_pipe_config_compare while checking
>scrambling status
> 
> V8: Fixed mege conflict, Addressed review comments from Ander
>  - Remove the desciption/comment about scrambling fom the caller, move
>it to the function
>  - Move the IS_GLK check into scrambling function
>  - Fix alignment
> 
> Signed-off-by: Shashank Sharma 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  7 
>  drivers/gpu/drm/i915/intel_ddi.c | 19 +++
>  drivers/gpu/drm/i915/intel_display.c |  3 ++
>  drivers/gpu/drm/i915/intel_drv.h | 10 ++
>  drivers/gpu/drm/i915/intel_hdmi.c| 62 
> 
>  5 files changed, 101 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc843f9..2d50fdc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7830,7 +7830,14 @@ enum {
>  #define  TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12)
>  #define  TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12)
>  #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC   (1<<8)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
>  #define  TRANS_DDI_BFI_ENABLE(1<<4)
> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE   (1<<4)
> +#define  TRANS_DDI_HDMI_SCRAMBLING   (1<<0)
> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE 
> \
> + | TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> + | TRANS_DDI_HDMI_SCRAMBLING)
>  
>  /* DisplayPort Transport Control */
>  #define _DP_TP_CTL_A 0x64040
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 0467676..b3e4c4a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct 
> intel_crtc_state *crtc_state)
>   temp |= TRANS_DDI_MODE_SELECT_HDMI;
>   else
>   temp |= TRANS_DDI_MODE_SELECT_DVI;
> +
> + if (crtc_state->hdmi_scrambling)
> + temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> + if (crtc_state->hdmi_high_tmds_clock_ratio)
> + temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
>   } else if (type == INTEL_OUTPUT_ANALOG) {
>   temp |= TRANS_DDI_MODE_SELECT_FDI;
>   temp |= (crtc_state->fdi_lanes - 1) << 1;
> @@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder 
> *intel_encoder,
>   struct intel_digital_port *intel_dig_port =
>   enc_to_dig_port(encoder);
>  
> + intel_hdmi_handle_sink_scrambling(intel_encoder,
> + 

Re: [PATCH v7 5/6] drm/i915: enable scrambling

[Ander Conselvan De Oliveira]

On Fri, 2017-03-03 at 21:58 +0530, Shashank Sharma wrote:
> Geminilake platform sports a native HDMI 2.0 controller, and is
> capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
> mendates scrambling for these higher clocks, for reduced RF footprint.
> 
> This patch checks if the monitor supports scrambling, and if required,
> enables it during the modeset.
> 
> V2: Addressed review comments from Ville:
> - Do not track scrambling status in DRM layer, track somewhere in
>   driver like in intel_crtc_state.
> - Don't talk to monitor at such a low layer, set monitor scrambling
>   in intel_enable_ddi() before enabling the port.
> 
> V3: Addressed review comments from Jani
>  - In comments, function names, use "sink" instead of "monitor",
>so that the implementation could be close to the language of
>HDMI spec.
> 
> V4: Addressed review comment from Maarten
>  - scrambling -> hdmi_scrambling
>high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> 
> V5: Addressed review comments from Ville and Ander
>  - Do not modifiy the crtc_state after compute_config. Move all
>scrambling and tmds_clock_ratio calcutations to compute_config.
>  - While setting scrambling for source/sink, do not check the
>conditions again, just go by the crtc_state flags. This will
>simplyfy the condition checks.
> 
> V6: Addressed review comments from Ville
>  - Do not add IS_GLK check in disable/enable function, instead add it
>in compute_config, while setting state flags.
>  - Remove unnecessary paranthesis.
>  - Simplyfy handle_sink_scrambling function as suggested.
>  - Add readout code for scrambling status in get_ddi_config and add a
>check for the same in pipe_config_compare.
> 
> V7: Addressed review comments from Ander/Ville
>  - No separate function for source scrambling, make it inline
>  - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
>  - Do not add platform check while setting source scrambling
>  - Use pipe_config instead of crtc->config to set sink scrambling
>  - To readout scrambling status, Compare with SCRAMBLING_MASK
>not any of its bits
>  - Remove platform check in intel_pipe_config_compare while checking
>scrambling status
> 
> Signed-off-by: Shashank Sharma 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  7 +
>  drivers/gpu/drm/i915/intel_ddi.c | 33 +++
>  drivers/gpu/drm/i915/intel_display.c |  3 +++
>  drivers/gpu/drm/i915/intel_drv.h | 10 +++
>  drivers/gpu/drm/i915/intel_hdmi.c| 52 
> 
>  5 files changed, 105 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4906ce4d..bfd988b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7824,7 +7824,14 @@ enum {
>  #define  TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12)
>  #define  TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12)
>  #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC   (1<<8)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
>  #define  TRANS_DDI_BFI_ENABLE(1<<4)
> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE   (1<<4)
> +#define  TRANS_DDI_HDMI_SCRAMBLING   (1<<0)
> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE 
> \
> + | TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> + | TRANS_DDI_HDMI_SCRAMBLING)
>  
>  /* DisplayPort Transport Control */
>  #define _DP_TP_CTL_A 0x64040
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index a7c08d7..2159b2b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1311,6 +1311,11 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc 
> *crtc)
>   temp |= TRANS_DDI_MODE_SELECT_HDMI;
>   else
>   temp |= TRANS_DDI_MODE_SELECT_DVI;
> +
> + if (intel_crtc->config->hdmi_scrambling)
> + temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> + if (intel_crtc->config->hdmi_high_tmds_clock_ratio)
> + temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
>   } else if (type == INTEL_OUTPUT_ANALOG) {
>   temp |= TRANS_DDI_MODE_SELECT_FDI;
>   temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
> @@ -1885,6 +1890,20 @@ static void intel_enable_ddi(struct intel_encoder 
> *intel_encoder,
>   struct intel_digital_port *intel_dig_port =
>   enc_to_dig_port(encoder);
>  
> + if (IS_GEMINILAKE(dev_priv)) {
> + /*
> +  * GLK sports a native HDMI 2.0 controller. If required
> +  * clock rate is > 340 Mhz && scrambling is supported
> +  * by sink, enable scrambling before enabling the
> +  * 

Re: [Intel-gfx] [PATCH v6 5/6] drm/i915: enable scrambling

[Ander Conselvan De Oliveira]

On Fri, 2017-03-03 at 17:33 +0530, Sharma, Shashank wrote:
> Thanks for the review Ander. My comments inline.
> 
> Shashank
> 
> On 3/3/2017 3:22 PM, Ander Conselvan De Oliveira wrote:
> > On Fri, 2017-03-03 at 11:59 +0530, Shashank Sharma wrote:
> > > Geminilake platform sports a native HDMI 2.0 controller, and is
> > > capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
> > > mendates scrambling for these higher clocks, for reduced RF footprint.
> > > 
> > > This patch checks if the monitor supports scrambling, and if required,
> > > enables it during the modeset.
> > > 
> > > V2: Addressed review comments from Ville:
> > > - Do not track scrambling status in DRM layer, track somewhere in
> > >driver like in intel_crtc_state.
> > > - Don't talk to monitor at such a low layer, set monitor scrambling
> > >in intel_enable_ddi() before enabling the port.
> > > 
> > > V3: Addressed review comments from Jani
> > >   - In comments, function names, use "sink" instead of "monitor",
> > > so that the implementation could be close to the language of
> > > HDMI spec.
> > > 
> > > V4: Addressed review comment from Maarten
> > >   - scrambling -> hdmi_scrambling
> > > high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> > > 
> > > V5: Addressed review comments from Ville and Ander
> > >   - Do not modifiy the crtc_state after compute_config. Move all
> > > scrambling and tmds_clock_ratio calcutations to compute_config.
> > >   - While setting scrambling for source/sink, do not check the
> > > conditions again, just go by the crtc_state flags. This will
> > > simplyfy the condition checks.
> > > 
> > > V6: Addressed review comments from Ville
> > >   - Do not add IS_GLK check in disable/enable function, instead add it
> > > in compute_config, while setting state flags.
> > >   - Remove unnecessary paranthesis.
> > >   - Simplyfy handle_sink_scrambling function as suggested.
> > >   - Add readout code for scrambling status in get_ddi_config and add a
> > > check for the same in pipe_config_compare.
> > > 
> > > Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/i915_reg.h  |  7 
> > >   drivers/gpu/drm/i915/intel_ddi.c | 33 
> > >   drivers/gpu/drm/i915/intel_display.c |  5 +++
> > >   drivers/gpu/drm/i915/intel_drv.h | 14 +++
> > >   drivers/gpu/drm/i915/intel_hdmi.c| 74 
> > > 
> > >   5 files changed, 133 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 4906ce4d..f7891ac 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7824,7 +7824,14 @@ enum {
> > >   #define  TRANS_DDI_EDP_INPUT_B_ONOFF(5<<12)
> > >   #define  TRANS_DDI_EDP_INPUT_C_ONOFF(6<<12)
> > >   #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC  (1<<8)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
> > >   #define  TRANS_DDI_BFI_ENABLE   (1<<4)
> > > +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE   (1<<4)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLING   (1<<0)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK 
> > > (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
> > > + | TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> > > + | TRANS_DDI_HDMI_SCRAMBLING)
> > 
> > Last line is misaligned.
> 
> Ok.
> > >   
> > >   /* DisplayPort Transport Control */
> > >   #define _DP_TP_CTL_A0x64040
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index a7c08d7..d0c6927 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -1311,6 +1311,11 @@ void intel_ddi_enable_transcoder_func(struct 
> > > drm_crtc *crtc)
> > >   temp |= TRANS_DDI_MODE_SELECT_HDMI;
> > >   else
> > >   temp |= TRANS_DDI_MODE_SELECT_DVI;
> > > +
> > > + if (IS_GEMIN

Re: [Intel-gfx] [PATCH v6 5/6] drm/i915: enable scrambling

[Ander Conselvan De Oliveira]

On Fri, 2017-03-03 at 11:59 +0530, Shashank Sharma wrote:
> Geminilake platform sports a native HDMI 2.0 controller, and is
> capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
> mendates scrambling for these higher clocks, for reduced RF footprint.
> 
> This patch checks if the monitor supports scrambling, and if required,
> enables it during the modeset.
> 
> V2: Addressed review comments from Ville:
> - Do not track scrambling status in DRM layer, track somewhere in
>   driver like in intel_crtc_state.
> - Don't talk to monitor at such a low layer, set monitor scrambling
>   in intel_enable_ddi() before enabling the port.
> 
> V3: Addressed review comments from Jani
>  - In comments, function names, use "sink" instead of "monitor",
>so that the implementation could be close to the language of
>HDMI spec.
> 
> V4: Addressed review comment from Maarten
>  - scrambling -> hdmi_scrambling
>high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> 
> V5: Addressed review comments from Ville and Ander
>  - Do not modifiy the crtc_state after compute_config. Move all
>scrambling and tmds_clock_ratio calcutations to compute_config.
>  - While setting scrambling for source/sink, do not check the
>conditions again, just go by the crtc_state flags. This will
>simplyfy the condition checks.
> 
> V6: Addressed review comments from Ville
>  - Do not add IS_GLK check in disable/enable function, instead add it
>in compute_config, while setting state flags.
>  - Remove unnecessary paranthesis.
>  - Simplyfy handle_sink_scrambling function as suggested.
>  - Add readout code for scrambling status in get_ddi_config and add a
>check for the same in pipe_config_compare.
> 
> Signed-off-by: Shashank Sharma 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  7 
>  drivers/gpu/drm/i915/intel_ddi.c | 33 
>  drivers/gpu/drm/i915/intel_display.c |  5 +++
>  drivers/gpu/drm/i915/intel_drv.h | 14 +++
>  drivers/gpu/drm/i915/intel_hdmi.c| 74 
> 
>  5 files changed, 133 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4906ce4d..f7891ac 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7824,7 +7824,14 @@ enum {
>  #define  TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12)
>  #define  TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12)
>  #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC   (1<<8)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
>  #define  TRANS_DDI_BFI_ENABLE(1<<4)
> +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE   (1<<4)
> +#define  TRANS_DDI_HDMI_SCRAMBLING   (1<<0)
> +#define  TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE 
> \
> + | TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> + | TRANS_DDI_HDMI_SCRAMBLING)

Last line is misaligned.

>  
>  /* DisplayPort Transport Control */
>  #define _DP_TP_CTL_A 0x64040
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index a7c08d7..d0c6927 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1311,6 +1311,11 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc 
> *crtc)
>   temp |= TRANS_DDI_MODE_SELECT_HDMI;
>   else
>   temp |= TRANS_DDI_MODE_SELECT_DVI;
> +
> + if (IS_GEMINILAKE(dev_priv))
> + temp = intel_hdmi_handle_source_scrambling(
> + intel_encoder,
> + intel_crtc->config, temp);

This whole hunk could just be:

+   if (config->hdmi_scrambling)
+   hdmi_config |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
+
+   if (config->hdmi_high_tmds_clock_ratio)
+   hdmi_config |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;

No need to check for GLK here, since the compute config code already did.

>   } else if (type == INTEL_OUTPUT_ANALOG) {
>   temp |= TRANS_DDI_MODE_SELECT_FDI;
>   temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
> @@ -1885,6 +1890,21 @@ static void intel_enable_ddi(struct intel_encoder 
> *intel_encoder,
>   struct intel_digital_port *intel_dig_port =
>   enc_to_dig_port(encoder);
>  
> + if (IS_GEMINILAKE(dev_priv)) {
> + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> + /*
> +  * GLK sports a native HDMI 2.0 controller. If required
> +  * clock rate is > 340 Mhz && scrambling is supported
> +  * by sink, enable scrambling before enabling the
> +  * HDMI 2.0 port. The sink can choose to disable the
> +  * scrambling if it doesn't 

Re: [Intel-gfx] [PATCH v5 5/6] drm/i915: enable scrambling

[Ander Conselvan De Oliveira]

On Thu, 2017-03-02 at 08:25 +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 3/1/2017 8:41 PM, Ville Syrjälä wrote:
> > On Tue, Feb 28, 2017 at 02:09:09PM +0530, Shashank Sharma wrote:
> > > Geminilake platform sports a native HDMI 2.0 controller, and is
> > > capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
> > > mendates scrambling for these higher clocks, for reduced RF footprint.
> > > 
> > > This patch checks if the monitor supports scrambling, and if required,
> > > enables it during the modeset.
> > > 
> > > V2: Addressed review comments from Ville:
> > > - Do not track scrambling status in DRM layer, track somewhere in
> > >driver like in intel_crtc_state.
> > > - Don't talk to monitor at such a low layer, set monitor scrambling
> > >in intel_enable_ddi() before enabling the port.
> > > 
> > > V3: Addressed review comments from Jani
> > >   - In comments, function names, use "sink" instead of "monitor",
> > > so that the implementation could be close to the language of
> > > HDMI spec.
> > > 
> > > V4: Addressed review comment from Maarten
> > >   - scrambling -> hdmi_scrambling
> > > high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> > > 
> > > V5: Addressed review comments from Ville and Ander
> > >   - Do not modifiy the crtc_state after compute_config. Move all
> > > scrambling and tmds_clock_ratio calcutations to compute_config.
> > >   - While setting scrambling for source/sink, do not check the
> > > conditions again, just go by the crtc_state flags. This will
> > > simplyfy the condition checks.
> > > 
> > > Signed-off-by: Shashank Sharma 
> > > ---
> > >   drivers/gpu/drm/i915/i915_reg.h   |  4 ++
> > >   drivers/gpu/drm/i915/intel_ddi.c  | 29 
> > >   drivers/gpu/drm/i915/intel_drv.h  | 14 ++
> > >   drivers/gpu/drm/i915/intel_hdmi.c | 98 
> > > +++
> > >   4 files changed, 145 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index ce6f096..1759b64 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7824,7 +7824,11 @@ enum {
> > >   #define  TRANS_DDI_EDP_INPUT_B_ONOFF(5<<12)
> > >   #define  TRANS_DDI_EDP_INPUT_C_ONOFF(6<<12)
> > >   #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC  (1<<8)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
> > >   #define  TRANS_DDI_BFI_ENABLE   (1<<4)
> > > +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE   (1<<4)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLING   (1<<0)
> > >   
> > >   /* DisplayPort Transport Control */
> > >   #define _DP_TP_CTL_A0x64040
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index a7c08d7..24bc3a8 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -1311,6 +1311,11 @@ void intel_ddi_enable_transcoder_func(struct 
> > > drm_crtc *crtc)
> > >   temp |= TRANS_DDI_MODE_SELECT_HDMI;
> > >   else
> > >   temp |= TRANS_DDI_MODE_SELECT_DVI;
> > > +
> > > + if (IS_GEMINILAKE(dev_priv))
> > > + temp = intel_hdmi_handle_source_scrambling(
> > > + intel_encoder,
> > > + intel_crtc->config, temp);
> > >   } else if (type == INTEL_OUTPUT_ANALOG) {
> > >   temp |= TRANS_DDI_MODE_SELECT_FDI;
> > >   temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
> > > @@ -1885,6 +1890,21 @@ static void intel_enable_ddi(struct intel_encoder 
> > > *intel_encoder,
> > >   struct intel_digital_port *intel_dig_port =
> > >   enc_to_dig_port(encoder);
> > >   
> > > + if (IS_GEMINILAKE(dev_priv)) {
> > > + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> > > + /*
> > > +  * GLK sports a native HDMI 2.0 controller. If required
> > > +  * clock rate is > 340 Mhz && scrambling is supported
> > > +  * by sink, enable scrambling before enabling the
> > > +  * HDMI 2.0 port. The sink can choose to disable the
> > > +  * scrambling if it doesn't detect a scrambled within
> > > +  * 100 ms.
> > > +  */
> > > + intel_hdmi_handle_sink_scrambling(intel_encoder,
> > > + conn_state->connector,
> > > + crtc->config, true);
> > > + }
> > > +
> > >   /* In HDMI/DVI mode, the port width, and swing/emphasis 
> > > values
> > >* are ignored so nothing special needs to be done 
> > > besides
> > >* enabling 

Re: [PATCH v4 5/6] drm/i915: enable scrambling

[Ander Conselvan De Oliveira]

On Thu, 2017-02-23 at 16:33 +0530, Sharma, Shashank wrote:
> Thanks for the review Ander, my comments, inline.
> 
> 
> Regards
> 
> Shashank
> 
> 
> On 2/23/2017 1:33 PM, Ander Conselvan De Oliveira wrote:
> > On Thu, 2017-02-23 at 10:01 +0530, Sharma, Shashank wrote:
> > > Regards
> > > 
> > > Shashank
> > > 
> > > 
> > > On 2/22/2017 10:59 PM, Ville Syrjälä wrote:
> > > > On Wed, Feb 22, 2017 at 06:48:30PM +0530, Shashank Sharma wrote:
> > > > > Geminilake platform sports a native HDMI 2.0 controller, and is
> > > > > capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
> > > > > mendates scrambling for these higher clocks, for reduced RF footprint.
> > > > > 
> > > > > This patch checks if the monitor supports scrambling, and if required,
> > > > > enables it during the modeset.
> > > > > 
> > > > > V2: Addressed review comments from Ville:
> > > > > - Do not track scrambling status in DRM layer, track somewhere in
> > > > > driver like in intel_crtc_state.
> > > > > - Don't talk to monitor at such a low layer, set monitor scrambling
> > > > > in intel_enable_ddi() before enabling the port.
> > > > > 
> > > > > V3: Addressed review comments from Jani
> > > > > - In comments, function names, use "sink" instead of "monitor",
> > > > > so that the implementation could be close to the language of
> > > > > HDMI spec.
> > > > > 
> > > > > V4: Addressed review comment from Maarten
> > > > > - scrambling -> hdmi_scrambling
> > > > > high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> > > > > 
> > > > > Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> > > > > ---
> > > > >drivers/gpu/drm/i915/i915_reg.h   |   4 ++
> > > > >drivers/gpu/drm/i915/intel_ddi.c  |  28 ++
> > > > >drivers/gpu/drm/i915/intel_drv.h  |  14 +
> > > > >drivers/gpu/drm/i915/intel_hdmi.c | 108 
> > > > > ++
> > > > >4 files changed, 154 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > index 141a5c1..81cf10b 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > @@ -7819,7 +7819,11 @@ enum {
> > > > >#define  TRANS_DDI_EDP_INPUT_B_ONOFF   (5<<12)
> > > > >#define  TRANS_DDI_EDP_INPUT_C_ONOFF   (6<<12)
> > > > >#define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8)
> > > > > +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> > > > > +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
> > > > >#define  TRANS_DDI_BFI_ENABLE  (1<<4)
> > > > > +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE   (1<<4)
> > > > > +#define  TRANS_DDI_HDMI_SCRAMBLING   (1<<0)
> > > > >
> > > > >/* DisplayPort Transport Control */
> > > > >#define _DP_TP_CTL_A   0x64040
> > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > > > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > > > index cd6fedd..bd8293d 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > > @@ -1278,6 +1278,11 @@ void intel_ddi_enable_transcoder_func(struct 
> > > > > drm_crtc *crtc)
> > > > >   temp |= TRANS_DDI_MODE_SELECT_HDMI;
> > > > >   else
> > > > >   temp |= TRANS_DDI_MODE_SELECT_DVI;
> > > > > +
> > > > > + if (IS_GEMINILAKE(dev_priv))
> > > > > + temp = intel_hdmi_handle_source_scrambling(
> > > > > + intel_encoder,
> > > > > + 
> > > > > _crtc->config->base.adjusted_mode, temp);
> > > > >   } else if (type == INTEL_OUTPUT_ANALOG) {
> > > > >   temp |= TRANS

Re: [PATCH v4 5/6] drm/i915: enable scrambling

[Ander Conselvan De Oliveira]

On Thu, 2017-02-23 at 10:01 +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 2/22/2017 10:59 PM, Ville Syrjälä wrote:
> > On Wed, Feb 22, 2017 at 06:48:30PM +0530, Shashank Sharma wrote:
> > > Geminilake platform sports a native HDMI 2.0 controller, and is
> > > capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec
> > > mendates scrambling for these higher clocks, for reduced RF footprint.
> > > 
> > > This patch checks if the monitor supports scrambling, and if required,
> > > enables it during the modeset.
> > > 
> > > V2: Addressed review comments from Ville:
> > > - Do not track scrambling status in DRM layer, track somewhere in
> > >driver like in intel_crtc_state.
> > > - Don't talk to monitor at such a low layer, set monitor scrambling
> > >in intel_enable_ddi() before enabling the port.
> > > 
> > > V3: Addressed review comments from Jani
> > > - In comments, function names, use "sink" instead of "monitor",
> > >so that the implementation could be close to the language of
> > >HDMI spec.
> > > 
> > > V4: Addressed review comment from Maarten
> > > - scrambling -> hdmi_scrambling
> > >high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio
> > > 
> > > Signed-off-by: Shashank Sharma 
> > > ---
> > >   drivers/gpu/drm/i915/i915_reg.h   |   4 ++
> > >   drivers/gpu/drm/i915/intel_ddi.c  |  28 ++
> > >   drivers/gpu/drm/i915/intel_drv.h  |  14 +
> > >   drivers/gpu/drm/i915/intel_hdmi.c | 108 
> > > ++
> > >   4 files changed, 154 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 141a5c1..81cf10b 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7819,7 +7819,11 @@ enum {
> > >   #define  TRANS_DDI_EDP_INPUT_B_ONOFF(5<<12)
> > >   #define  TRANS_DDI_EDP_INPUT_C_ONOFF(6<<12)
> > >   #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC  (1<<8)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6)
> > >   #define  TRANS_DDI_BFI_ENABLE   (1<<4)
> > > +#define  TRANS_DDI_HIGH_TMDS_CHAR_RATE   (1<<4)
> > > +#define  TRANS_DDI_HDMI_SCRAMBLING   (1<<0)
> > >   
> > >   /* DisplayPort Transport Control */
> > >   #define _DP_TP_CTL_A0x64040
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index cd6fedd..bd8293d 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -1278,6 +1278,11 @@ void intel_ddi_enable_transcoder_func(struct 
> > > drm_crtc *crtc)
> > >   temp |= TRANS_DDI_MODE_SELECT_HDMI;
> > >   else
> > >   temp |= TRANS_DDI_MODE_SELECT_DVI;
> > > +
> > > + if (IS_GEMINILAKE(dev_priv))
> > > + temp = intel_hdmi_handle_source_scrambling(
> > > + intel_encoder,
> > > + _crtc->config->base.adjusted_mode, temp);
> > >   } else if (type == INTEL_OUTPUT_ANALOG) {
> > >   temp |= TRANS_DDI_MODE_SELECT_FDI;
> > >   temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
> > > @@ -1843,6 +1848,21 @@ static void intel_enable_ddi(struct intel_encoder 
> > > *intel_encoder,
> > >   struct intel_digital_port *intel_dig_port =
> > >   enc_to_dig_port(encoder);
> > >   
> > > + if (IS_GEMINILAKE(dev_priv)) {
> > > + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> > > + /*
> > > +  * GLK sports a native HDMI 2.0 controller. If required
> > > +  * clock rate is > 340 Mhz && scrambling is supported
> > > +  * by sink, enable scrambling before enabling the
> > > +  * HDMI 2.0 port. The sink can choose to disable the
> > > +  * scrambling if it doesn't detect a scrambled within
> > > +  * 100 ms.
> > > +  */
> > > + intel_hdmi_handle_sink_scrambling(intel_encoder,
> > > + conn_state->connector,
> > > + crtc->config, true);
> > > + }
> > > +
> > >   /* In HDMI/DVI mode, the port width, and swing/emphasis 
> > > values
> > >* are ignored so nothing special needs to be done 
> > > besides
> > >* enabling the port.
> > > @@ -1875,6 +1895,14 @@ static void intel_disable_ddi(struct intel_encoder 
> > > *intel_encoder,
> > >   if (old_crtc_state->has_audio)
> > >   intel_audio_codec_disable(intel_encoder);
> > >   
> > > + if (type == INTEL_OUTPUT_HDMI) {
> > > + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> > > +
> > > + 

[PATCH] drm: Add name for DRM_DP_DUAL_MODE_LSPCON

[Ander Conselvan de Oliveira]

Handle DRM_DP_DUAL_MODE_LSPCON in drm_dp_get_dual_mode_type_name(),
otherwise a call to that function can theoretically trigger a WARN.

Fixes: 056996b95686 ("drm: Helper for lspcon in drm_dp_dual_mode")
Cc: Shashank Sharma <shashank.sha...@intel.com>
Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
Cc: Dave Airlie <airl...@gmail.com>
Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
Cc: Jani Nikula <jani.nik...@intel.com>
Cc: Daniel Vetter <daniel.vet...@intel.com>
Cc: Jani Nikula <jani.nik...@linux.intel.com>
Cc: Sean Paul <seanp...@chromium.org>
Cc: David Airlie <airl...@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Signed-off-by: Ander Conselvan de Oliveira 
<ander.conselvan.de.olive...@intel.com>
---
 drivers/gpu/drm/drm_dp_dual_mode_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c 
b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
index e025639..80e62f6 100644
--- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
@@ -386,6 +386,8 @@ const char *drm_dp_get_dual_mode_type_name(enum 
drm_dp_dual_mode_type type)
return "type 2 DVI";
case DRM_DP_DUAL_MODE_TYPE2_HDMI:
return "type 2 HDMI";
+   case DRM_DP_DUAL_MODE_LSPCON:
+   return "lspcon";
default:
WARN_ON(type != DRM_DP_DUAL_MODE_UNKNOWN);
return "unknown";
-- 
2.9.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH v3 6/6] drm/i915: allow HDMI 2.0 clock rates

[Ander Conselvan De Oliveira]

On Fri, 2017-02-10 at 21:59 +0530, Shashank Sharma wrote:
> Geminilake has a native HDMI 2.0 controller, which is capable of
> driving clocks upto 594Mhz. This patch updates the max tmds clock
> limit for the same.
> 
> V2: rebase
> V3: rebase
> 
> Cc: Ander Conselvan De Oliveira <ander.conselvan.de.olive...@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 9970131..b1ddcc5 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1210,6 +1210,8 @@ static int intel_hdmi_source_max_tmds_clock(struct 
> drm_i915_private *dev_priv)
>  {
>   if (IS_G4X(dev_priv))
>   return 165000;
> + else if (IS_GEMINILAKE(dev_priv))
> + return 594000;
>   else if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
>   return 30;
>   else

Reviewed-by: Ander Conselvan de Oliveira <conselv...@gmail.com>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH v2 1/2] drm: Wrap the check for atomic_commit implementation

[Ander Conselvan De Oliveira]

On Wed, 2016-12-21 at 12:12 -0800, Dhinakaran Pandiyan wrote:
> This check is useful for drivers that do not have DRIVER_ATOMIC set but
> have atomic modesetting internally implemented. Wrap the check into a
> function since this is used in many places and as a bonus, the function
> name helps to document what the check is for.
> 
> v2:
> Change return type to bool (Ville)
> Move the function drm_atomic.h (Daniel)
> 
> Suggested-by: Daniel Vetter 
> Cc: Daniel Vetter 
> Cc: Ben Skeggs 
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/drm_fb_helper.c             |  6 +++---
>  drivers/gpu/drm/nouveau/nouveau_connector.c |  5 +++--
>  drivers/gpu/drm/nouveau/nouveau_display.c   |  6 +++---
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  3 ++-
>  include/drm/drm_atomic.h                    | 11 
> +++
>  5 files changed, 22 insertions(+), 9 deletions(-)
> 

...

> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 8cc7ca2..43db162 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -419,5 +419,16 @@ drm_atomic_crtc_needs_modeset(const struct drm_crtc_state
> *state)
>         state->connectors_changed;
>  }
>  
> +/* drm_drv_uses_atomic_modeset - check if the driver implements

Shouldn't this be

/**
 * drm_drv_uses_atomic_modeset - ...

so it is included in the generated documentation?

Ander


> + * atomic_commit()
> + * @dev: DRM device
> + *
> + * This check is useful if drivers do not have DRIVER_ATOMIC set but
> + * have atomic modesetting internally implemented.
> + */
> +static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
> +{
> + return dev->mode_config.funcs->atomic_commit ? true : false;
> +}
>  
>  #endif /* DRM_ATOMIC_H_ */

[PATCH 10/10] drm/i915: Account for sink max TMDS clock when checking the port clock

[Ander Conselvan De Oliveira]

On Wed, 2016-09-28 at 16:51 +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> It's perfectly legal for the sink to support 12bpc only for
> some lower resolution modes, while the higher resolution modes
> can only be used with 8bpc. So let's take the sink's max TMDS clock
> into account before we go and decide that a particular mode can
> be used with 12bpc.

Reviewed-by: Ander Conselvan de Oliveira 

> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8d49800064df..8d46f5836746 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1220,10 +1220,17 @@ static int hdmi_port_clock_limit(struct intel_hdmi
> *hdmi,
>  int max_tmds_clock = intel_hdmi_source_max_tmds_clock(to_i915(dev));
>  
>  if (respect_downstream_limits) {
> + struct intel_connector *connector = hdmi->attached_connector;
> + const struct drm_display_info *info = 
> >base.display_info;
> +
>  if (hdmi->dp_dual_mode.max_tmds_clock)
>  max_tmds_clock = min(max_tmds_clock,
>       hdmi-
> >dp_dual_mode.max_tmds_clock);
> - if (!hdmi->has_hdmi_sink)
> +
> + if (info->max_tmds_clock)
> + max_tmds_clock = min(max_tmds_clock,
> +      info->max_tmds_clock);
> + else if (!hdmi->has_hdmi_sink)
>  max_tmds_clock = min(max_tmds_clock, 165000);
>  }
>  

[PATCH 09/10] drm/i915: Replace a bunch of connector->base.display_info with a local variable

[Ander Conselvan De Oliveira]

On Wed, 2016-09-28 at 16:51 +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Reduce the eyesore with a local variable.
> 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Ander Conselvan de Oliveira 

> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 8e464e089794..34ca03e621ba 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12657,22 +12657,22 @@ static void
>  connected_sink_compute_bpp(struct intel_connector *connector,
>     struct intel_crtc_state *pipe_config)
>  {
> + const struct drm_display_info *info = >base.display_info;
>  int bpp = pipe_config->pipe_bpp;
>  
>  DRM_DEBUG_KMS("[CONNECTOR:%d:%s] checking for sink bpp constrains\n",
> - connector->base.base.id,
> - connector->base.name);
> +       connector->base.base.id,
> +       connector->base.name);
>  
>  /* Don't use an invalid EDID bpc value */
> - if (connector->base.display_info.bpc &&
> -     connector->base.display_info.bpc * 3 < bpp) {
> + if (info->bpc != 0 && info->bpc * 3 < bpp) {
>  DRM_DEBUG_KMS("clamping display bpp (was %d) to EDID reported
> max of %d\n",
> -       bpp, connector->base.display_info.bpc*3);
> - pipe_config->pipe_bpp = connector->base.display_info.bpc*3;
> +       bpp, info->bpc * 3);
> + pipe_config->pipe_bpp = info->bpc * 3;
>  }
>  
>  /* Clamp bpp to 8 on screens without EDID 1.4 */
> - if (connector->base.display_info.bpc == 0 && bpp > 24) {
> + if (info->bpc == 0 && bpp > 24) {
>  DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit
> of 24\n",
>        bpp);
>  pipe_config->pipe_bpp = 24;

[PATCH v2 02/11] drm/i915: Remove stallcheck special handling.

[Ander Conselvan De Oliveira]

On Mon, 2016-04-18 at 07:31 +0200, Maarten Lankhorst wrote:
> Op 15-04-16 om 09:07 schreef Ander Conselvan De Oliveira:
> > On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> > > Re-use unpin_work->pending, but also set vblank count before
> > > intel_mark_page_flip_active to be sure.
> > Be sure of what?
> > 
> > > Signed-off-by: Maarten Lankhorst 
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c  | 11 ++-
> > >  drivers/gpu/drm/i915/intel_display.c | 31 ---
> > >  drivers/gpu/drm/i915/intel_drv.h |  1 -
> > >  3 files changed, 18 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 9640738aabf2..df8073a2ffbe 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -582,9 +582,14 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> > > void *data)
> > >   seq_printf(m, "No flip due on pipe %c (plane
> > > %c)\n",
> > >  pipe, plane);
> > >   } else {
> > > + u32 pending;
> > >   u32 addr;
> > >  
> > > - if (atomic_read(>pending) <
> > > INTEL_FLIP_COMPLETE) {
> > > + pending = atomic_read(>pending);
> > > + if (pending == INTEL_FLIP_INACTIVE) {
> > > + seq_printf(m, "Flip ioctl preparing on
> > > pipe
> > > %c (plane %c)\n",
> > > +pipe, plane);
> > > + } else if (pending >= INTEL_FLIP_COMPLETE) {
> > >   seq_printf(m, "Flip queued on pipe %c
> > > (plane
> > > %c)\n",
> > >  pipe, plane);
> > >   } else {
> > > @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> > > void *data)
> > >  work->flip_queued_vblank,
> > >  work->flip_ready_vblank,
> > >  drm_crtc_vblank_count(>base));
> > > - if (work->enable_stall_check)
> > > - seq_puts(m, "Stall check enabled, ");
> > > - else
> > > - seq_puts(m, "Stall check waiting for page
> > > flip ioctl, ");
> > >   seq_printf(m, "%d prepares\n", atomic_read(
> > > ->pending));
> > >  
> > >   if (INTEL_INFO(dev)->gen >= 4)
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index f2be54a48727..618e034a7a5e 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct
> > > intel_mmio_flip
> > > *mmio_flip)
> > >   if (work == NULL)
> > >   return;
> > >  
> > > - intel_mark_page_flip_active(work);
> > > -
> > >   intel_pipe_update_start(crtc);
> > >  
> > >   if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> > > @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct
> > > intel_mmio_flip
> > > *mmio_flip)
> > >   ilk_do_mmio_flip(crtc, work);
> > >  
> > >   intel_pipe_update_end(crtc);
> > > +
> > > + intel_mark_page_flip_active(work);
> > Is this to avoid triggering the stall check during the wait from a vblank
> > evasion?
> It's to ensure that if a vblank happens before pipe_update_end, we don't mark 
> the flip as completed until we actually updated the mmio registers.

But interrupts are disabled between pipe_update_start() and pipe_update_end(),
so if that happens it either happens before or during pipe_update_start(), no?

Is it possible the vblank happens just after pipe_update_end() and before
marking it active? Seems to me that in that case, first prepare_page_flip() will
increase unpin_work->pending (so it will go from INACTIVE to PENDING) and then
marking it active will set it again to PENDING, so it never gets to COMPLETE.

But even if the above can happen, that is fixed by the removal of the COMPLETE
state in patch 3.

> > 
> > >  }
> &g

[PATCH v2 05/11] drm/i915: Allow mmio updates on all platforms, v2.

[Ander Conselvan De Oliveira]

On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
> and unpinning.

I think the rename should be a separate patch.

Ander

>  Use flip_queued_req to hold the wait request in the
> mmio case and allow the vblank interrupt to complete mmio work to
> have mmio flips run correctly on g4 and earlier.
> 
> Changes since v1:
> - Add smp_mb__after_atomic() to __intel_pageflip_stall_check,
>   to match the smp_mb__before_atomic() in pipe_update_end().
> - Check for cur != flip_queued_vblank in pageflip_stall_check.
> 
> Signed-off-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |   4 +-
>  drivers/gpu/drm/i915/intel_display.c | 271 +++---
> -
>  drivers/gpu/drm/i915/intel_drv.h |  18 +--
>  drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
>  4 files changed, 95 insertions(+), 206 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index c3b029e7bffd..5662cd5a1a9d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -574,10 +574,10 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
>   for_each_intel_crtc(dev, crtc) {
>   const char pipe = pipe_name(crtc->pipe);
>   const char plane = plane_name(crtc->plane);
> - struct intel_unpin_work *work;
> + struct intel_flip_work *work;
>  
>   spin_lock_irq(>event_lock);
> - work = crtc->unpin_work;
> + work = crtc->flip_work;
>   if (work == NULL) {
>   seq_printf(m, "No flip due on pipe %c (plane %c)\n",
>  pipe, plane);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f1a895153e64..b614f118b973 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -48,6 +48,11 @@
>  #include 
>  #include 
>  
> +static bool is_mmio_work(struct intel_flip_work *work)
> +{
> + return work->mmio_work.func;
> +}
> +
>  /* Primary plane formats for gen <= 3 */
>  static const uint32_t i8xx_primary_formats[] = {
>   DRM_FORMAT_C8,
> @@ -3285,7 +3290,7 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc
> *crtc)
>   return false;
>  
>   spin_lock_irq(>event_lock);
> - pending = to_intel_crtc(crtc)->unpin_work != NULL;
> + pending = to_intel_crtc(crtc)->flip_work != NULL;
>   spin_unlock_irq(>event_lock);
>  
>   return pending;
> @@ -3864,7 +3869,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
>   if (atomic_read(>unpin_work_count) == 0)
>   continue;
>  
> - if (crtc->unpin_work)
> + if (crtc->flip_work)
>   intel_wait_for_vblank(dev, crtc->pipe);
>  
>   return true;
> @@ -3876,11 +3881,11 @@ bool intel_has_pending_fb_unpin(struct drm_device
> *dev)
>  static void page_flip_completed(struct intel_crtc *intel_crtc)
>  {
>   struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> - struct intel_unpin_work *work = intel_crtc->unpin_work;
> + struct intel_flip_work *work = intel_crtc->flip_work;
>  
> - /* ensure that the unpin work is consistent wrt ->pending. */
> + /* ensure that the flip work is consistent wrt ->pending. */
>   smp_rmb();
> - intel_crtc->unpin_work = NULL;
> + intel_crtc->flip_work = NULL;
>  
>   if (work->event)
>   drm_send_vblank_event(intel_crtc->base.dev,
> @@ -3890,7 +3895,7 @@ static void page_flip_completed(struct intel_crtc
> *intel_crtc)
>   drm_crtc_vblank_put(_crtc->base);
>  
>   wake_up_all(_priv->pending_flip_queue);
> - queue_work(dev_priv->wq, >work);
> + queue_work(dev_priv->wq, >unpin_work);
>  
>   trace_i915_flip_complete(intel_crtc->plane,
>work->pending_flip_obj);
> @@ -3914,9 +3919,11 @@ static int intel_crtc_wait_for_pending_flips(struct
> drm_crtc *crtc)
>  
>   if (ret == 0) {
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_flip_work *work;
>  
>   spin_lock_irq(>event_lock);
> - if (intel_crtc->unpin_work) {
> + work = intel_crtc->flip_work;
> + if (work && !is_mmio_work(work)) {
>   WARN_ONCE(1, "Removing stuck page flip\n");
>   page_flip_completed(intel_crtc);
>   }
> @@ -6308,7 +6315,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc
> *crtc)
>   return;
>  
>   if (to_intel_plane_state(crtc->primary->state)->visible) {
> - WARN_ON(intel_crtc->unpin_work);
> + WARN_ON(intel_crtc->flip_work);
>  
>   intel_pre_disable_primary_noatomic(crtc);
>  
> @@ -10881,15 +10888,16 @@ 

[PATCH v2 03/11] drm/i915: Remove intel_prepare_page_flip.

[Ander Conselvan De Oliveira]

On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE
> is no longer useful.

What's the deal with "use atomic_read"? I couldn't find where that is different
from the old code.


> 
> Signed-off-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  3 --
>  drivers/gpu/drm/i915/i915_irq.c  | 18 ++-
>  drivers/gpu/drm/i915/intel_display.c | 96 ++-
> -
>  drivers/gpu/drm/i915/intel_drv.h |  2 -
>  4 files changed, 40 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index df8073a2ffbe..c3b029e7bffd 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -589,9 +589,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void
> *data)
>   if (pending == INTEL_FLIP_INACTIVE) {
>   seq_printf(m, "Flip ioctl preparing on pipe
> %c (plane %c)\n",
>  pipe, plane);
> - } else if (pending >= INTEL_FLIP_COMPLETE) {
> - seq_printf(m, "Flip queued on pipe %c (plane
> %c)\n",
> -pipe, plane);
>   } else {
>   seq_printf(m, "Flip pending (waiting for
> vsync) on pipe %c (plane %c)\n",
>  pipe, plane);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 679f08c944ef..5ed2f73a7ea9 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct
> drm_device *dev, u32 iir)
>   intel_pipe_handle_vblank(dev, pipe))
>   intel_check_page_flip(dev, pipe);
>  
> - if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV) {
> - intel_prepare_page_flip(dev, pipe);
> + if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
>   intel_finish_page_flip(dev, pipe);
> - }
>  
>   if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
>   i9xx_pipe_crc_irq_handler(dev, pipe);
> @@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device
> *dev, u32 de_iir)
>   i9xx_pipe_crc_irq_handler(dev, pipe);
>  
>   /* plane/pipes map 1:1 on ilk+ */
> - if (de_iir & DE_PLANE_FLIP_DONE(pipe)) {
> - intel_prepare_page_flip(dev, pipe);
> + if (de_iir & DE_PLANE_FLIP_DONE(pipe))
>   intel_finish_page_flip_plane(dev, pipe);
> - }
>   }
>  
>   /* check event from PCH */
> @@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device
> *dev, u32 de_iir)
>   intel_check_page_flip(dev, pipe);
>  
>   /* plane/pipes map 1:1 on ilk+ */
> - if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
> - intel_prepare_page_flip(dev, pipe);
> + if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
>   intel_finish_page_flip_plane(dev, pipe);
> - }
>   }
>  
>   /* check event from PCH */
> @@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv,
> u32 master_ctl)
>   else
>   flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
>  
> - if (flip_done) {
> - intel_prepare_page_flip(dev, pipe);
> + if (flip_done)
>   intel_finish_page_flip_plane(dev, pipe);
> - }
>  
>   if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>   hsw_pipe_crc_irq_handler(dev, pipe);
> @@ -4024,7 +4016,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
>   if (I915_READ16(ISR) & flip_pending)
>   goto check_page_flip;
>  
> - intel_prepare_page_flip(dev, plane);
>   intel_finish_page_flip(dev, pipe);
>   return true;
>  
> @@ -4215,7 +4206,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
>   if (I915_READ(ISR) & flip_pending)
>   goto check_page_flip;
>  
> - intel_prepare_page_flip(dev, plane);
>   intel_finish_page_flip(dev, pipe);
>   return true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 618e034a7a5e..dc42335409b3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3101,7 +3101,6 @@ static void intel_complete_page_flips(struct drm_device
> *dev)
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   enum plane plane = intel_crtc->plane;
>  
> - intel_prepare_page_flip(dev, plane);
>   

[PATCH v2 02/11] drm/i915: Remove stallcheck special handling.

[Ander Conselvan De Oliveira]

On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Re-use unpin_work->pending, but also set vblank count before
> intel_mark_page_flip_active to be sure.

Be sure of what?

> 
> Signed-off-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  | 11 ++-
>  drivers/gpu/drm/i915/intel_display.c | 31 ---
>  drivers/gpu/drm/i915/intel_drv.h |  1 -
>  3 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9640738aabf2..df8073a2ffbe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -582,9 +582,14 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
>   seq_printf(m, "No flip due on pipe %c (plane %c)\n",
>  pipe, plane);
>   } else {
> + u32 pending;
>   u32 addr;
>  
> - if (atomic_read(>pending) <
> INTEL_FLIP_COMPLETE) {
> + pending = atomic_read(>pending);
> + if (pending == INTEL_FLIP_INACTIVE) {
> + seq_printf(m, "Flip ioctl preparing on pipe
> %c (plane %c)\n",
> +pipe, plane);
> + } else if (pending >= INTEL_FLIP_COMPLETE) {
>   seq_printf(m, "Flip queued on pipe %c (plane
> %c)\n",
>  pipe, plane);
>   } else {
> @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
>  work->flip_queued_vblank,
>  work->flip_ready_vblank,
>  drm_crtc_vblank_count(>base));
> - if (work->enable_stall_check)
> - seq_puts(m, "Stall check enabled, ");
> - else
> - seq_puts(m, "Stall check waiting for page
> flip ioctl, ");
>   seq_printf(m, "%d prepares\n", atomic_read(
> ->pending));
>  
>   if (INTEL_INFO(dev)->gen >= 4)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f2be54a48727..618e034a7a5e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
> *mmio_flip)
>   if (work == NULL)
>   return;
>  
> - intel_mark_page_flip_active(work);
> -
>   intel_pipe_update_start(crtc);
>  
>   if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
> *mmio_flip)
>   ilk_do_mmio_flip(crtc, work);
>  
>   intel_pipe_update_end(crtc);
> +
> + intel_mark_page_flip_active(work);

Is this to avoid triggering the stall check during the wait from a vblank
evasion?


>  }
>  
>  static void intel_mmio_flip_work_func(struct work_struct *work)
> @@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct
> drm_device *dev,
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   struct intel_unpin_work *work = intel_crtc->unpin_work;
>   u32 addr;
> + u32 pending;
>  
> - if (atomic_read(>pending) >= INTEL_FLIP_COMPLETE)
> - return true;
> -
> - if (atomic_read(>pending) < INTEL_FLIP_PENDING)
> - return false;
> -
> - if (!work->enable_stall_check)
> - return false;
> + pending = atomic_read(>pending);
> + if (pending != INTEL_FLIP_PENDING)
> + return pending == INTEL_FLIP_COMPLETE;
>  
>   if (work->flip_ready_vblank == 0) {
>   if (work->flip_queued_req &&
> @@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>*/
>   if (!mmio_flip) {
>   ret = i915_gem_object_sync(obj, engine, );
> + if (!ret && !request) {
> + request = i915_gem_request_alloc(engine, NULL);
> + ret = PTR_ERR_OR_ZERO(request);
> + }
> +
>   if (ret)
>   goto cleanup_pending;
>   }
> @@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>   work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
> obj, 0);
>   work->gtt_offset += intel_crtc->dspaddr_offset;
> + work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>  
>   if (mmio_flip) {
>   ret = intel_queue_mmio_flip(dev, crtc, obj);
> @@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>   i915_gem_request_assign(>flip_queued_req,
>   obj->last_write_req);
>   

[PATCH] drm/dp: add eDP DPCD backlight control bit definitions

[Ander Conselvan De Oliveira]

On Thu, 2015-10-29 at 11:03 +0200, Jani Nikula wrote:
> Cc: Yetunde Adebisi 
> Signed-off-by: Jani Nikula 
> ---
>  include/drm/drm_dp_helper.h | 36 
>  1 file changed, 36 insertions(+)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index bb9d0deca07c..1252108da0ef 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -455,16 +455,52 @@
>  # define DP_EDP_14   0x03
>  
>  #define DP_EDP_GENERAL_CAP_1 0x701
> +# define DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP(1 << 0)
> +# define DP_EDP_BACKLIGHT_PIN_ENABLE_CAP (1 << 1)
> +# define DP_EDP_BACKLIGHT_AUX_ENABLE_CAP (1 << 2)
> +# define DP_EDP_PANEL_SELF_TEST_PIN_ENABLE_CAP   (1 << 3)
> +# define DP_EDP_PANEL_SELF_TEST_AUX_ENABLE_CAP   (1 << 4)
> +# define DP_EDP_FRC_ENABLE_CAP   (1 << 5)
> +# define DP_EDP_COLOR_ENGINE_CAP (1 << 6)
> +# define DP_EDP_SET_POWER_CAP(1 << 7)
>  
>  #define DP_EDP_BACKLIGHT_ADJUSTMENT_CAP 0x702
> +# define DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP (1 << 0)
> +# define DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP (1 << 1)
> +# define DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT  (1 << 2)
> +# define DP_EDP_BACKLIGHT_AUX_PWM_PRODUCT_CAP(1 << 3)
> +# define DP_EDP_BACKLIGHT_FREQ_PWM_PIN_PASSTHRU_CAP  (1 << 4)
> +# define DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP   (1 << 5)
> +# define DP_EDP_DYNAMIC_BACKLIGHT_CAP(1 << 6)
> +# define DP_EDP_VBLANK_BACKLIGHT_UPDATE_CAP  (1 << 7)
>  
>  #define DP_EDP_GENERAL_CAP_2 0x703
> +# define DP_EDP_OVERDRIVE_ENGINE_ENABLED (1 << 0)
>  
>  #define DP_EDP_GENERAL_CAP_3 0x704/* eDP 1.4 */
> +# define DP_EDP_X_REGION_CAP_MASK(0xf << 0)
> +# define DP_EDP_X_REGION_CAP_SHIFT   0
> +# define DP_EDP_Y_REGION_CAP_MASK(0xf << 4)
> +# define DP_EDP_Y_REGION_CAP_SHIFT   4
>  
>  #define DP_EDP_DISPLAY_CONTROL_REGISTER 0x720
> +# define DP_EDP_BACKLIGHT_ENABLE (1 << 0)
> +# define DP_EDP_BLACK_VIDEO_ENABLE   (1 << 1)
> +# define DP_EDP_FRC_ENABLE   (1 << 2)
> +# define DP_EDP_COLOR_ENGINE_ENABLE  (1 << 3)
> +# define DP_EDP_VBLANK_BACKLIGHT_UPDATE_ENABLE   (1 << 7)
>  
>  #define DP_EDP_BACKLIGHT_MODE_SET_REGISTER  0x721
> +# define DP_EDP_BACKLIGHT_CONTROL_MODE_MASK  (3 << 0)
> +# define DP_EDP_BACKLIGHT_CONTROL_MODE_PWM   (0 << 0)
> +# define DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET(1 << 0)
> +# define DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD  (2 << 0)
> +# define DP_EDP_BACKLIGHT_CONTROL_MODE_PRODUCT   (3 << 0)
> +# define DP_EDP_BACKLIGHT_FREQ_PWM_PIN_PASSTHRU_ENABLE   (1 << 2)
> +# define DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE(1 << 3)
> +# define DP_EDP_DYNAMIC_BACKLIGHT_ENABLE (1 << 4)
> +# define DP_EDP_REGIONAL_BACKLIGHT_ENABLE(1 << 5)

Doesn't DP_EDP_REGIONAL_BACKLIGHT_ENABLE need a /* eDP 1.4 */ comment too?

Anyway, it all matches the spec so,

Reviewed-by: Ander Conselvan de Oliveira 

> +# define DP_EDP_UPDATE_REGION_BRIGHTNESS (1 << 6) /* eDP 1.4
> */
>  
>  #define DP_EDP_BACKLIGHT_BRIGHTNESS_MSB 0x722
>  #define DP_EDP_BACKLIGHT_BRIGHTNESS_LSB 0x723

[PATCH] drm: Repurpose reserved field of drm_event_vlank to crtc_id

[Ander Conselvan de Oliveira]

With the atomic API, it is possible that a single commit affects
multiple crtcs. If the user requests an event with that commit, one
event will be sent for each CRTC, but it is not possible to distinguish
which crtc an event is for in user space. To solve this, the reserved
field in struct drm_vblank_event is repurposed to include the crtc_id
which the event is for.

The DRM_CAP_CRTC_IN_VBLANK_EVENT is added to allow userspace to query if
the crtc field will be set properly.

Signed-off-by: Ander Conselvan de Oliveira 
---
 drivers/gpu/drm/drm_atomic.c | 7 +--
 drivers/gpu/drm/drm_crtc.c   | 1 +
 drivers/gpu/drm/drm_ioctl.c  | 3 +++
 include/uapi/drm/drm.h   | 3 ++-
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1066e4b..2a76d10 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1304,7 +1304,8 @@ EXPORT_SYMBOL(drm_atomic_async_commit);
  */

 static struct drm_pending_vblank_event *create_vblank_event(
-   struct drm_device *dev, struct drm_file *file_priv, uint64_t 
user_data)
+   struct drm_device *dev, struct drm_file *file_priv,
+   uint64_t user_data, struct drm_crtc *crtc)
 {
struct drm_pending_vblank_event *e = NULL;
unsigned long flags;
@@ -1328,6 +1329,7 @@ static struct drm_pending_vblank_event 
*create_vblank_event(
e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
e->event.base.length = sizeof e->event;
e->event.user_data = user_data;
+   e->event.crtc_id = crtc->base.id;
e->base.event = >event.base;
e->base.file_priv = file_priv;
e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
@@ -1529,7 +1531,8 @@ retry:
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct drm_pending_vblank_event *e;

-   e = create_vblank_event(dev, file_priv, arg->user_data);
+   e = create_vblank_event(dev, file_priv, arg->user_data,
+   crtc);
if (!e) {
ret = -ENOMEM;
goto out;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 33d877c..98fe624 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5212,6 +5212,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
e->event.base.length = sizeof(e->event);
e->event.user_data = page_flip->user_data;
+   e->event.crtc_id = crtc->base.id;
e->base.event = >event.base;
e->base.file_priv = file_priv;
e->base.destroy =
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index b1d303f..152aeba 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -312,6 +312,9 @@ static int drm_getcap(struct drm_device *dev, void *data, 
struct drm_file *file_
case DRM_CAP_ADDFB2_MODIFIERS:
req->value = dev->mode_config.allow_fb_modifiers;
break;
+   case DRM_CAP_CRTC_IN_VBLANK_EVENT:
+   req->value = 1;
+   break;
default:
return -EINVAL;
}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 3801584..ecd9e96 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -631,6 +631,7 @@ struct drm_gem_open {
 #define DRM_CAP_CURSOR_WIDTH   0x8
 #define DRM_CAP_CURSOR_HEIGHT  0x9
 #define DRM_CAP_ADDFB2_MODIFIERS   0x10
+#define DRM_CAP_CRTC_IN_VBLANK_EVENT   0x11

 /** DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
@@ -826,7 +827,7 @@ struct drm_event_vblank {
__u32 tv_sec;
__u32 tv_usec;
__u32 sequence;
-   __u32 reserved;
+   __u32 crtc_id;
 };

 /* typedef area */
-- 
2.4.3

[PATCH libdrm] xf86drmMode: Handle the crtc_id field of drm_event_vblank

[Ander Conselvan de Oliveira]

Signed-off-by: Ander Conselvan de Oliveira 

---
 include/drm/drm.h |  2 +-
 xf86drm.h |  9 -
 xf86drmMode.c | 26 +-
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index 167b7b8..81f8af1 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -806,7 +806,7 @@ struct drm_event_vblank {
__u32 tv_sec;
__u32 tv_usec;
__u32 sequence;
-   __u32 reserved;
+   __u32 crtc_id;
 };

 #define DRM_CAP_DUMB_BUFFER 0x1
diff --git a/xf86drm.h b/xf86drm.h
index 360e04a..6eef8ac 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -726,7 +726,7 @@ extern void drmMsg(const char *format, ...) 
DRM_PRINTFLIKE(1, 2);
 extern int drmSetMaster(int fd);
 extern int drmDropMaster(int fd);

-#define DRM_EVENT_CONTEXT_VERSION 2
+#define DRM_EVENT_CONTEXT_VERSION 3

 typedef struct _drmEventContext {

@@ -746,6 +746,13 @@ typedef struct _drmEventContext {
  unsigned int tv_usec,
  void *user_data);

+   void (*page_flip_handler2)(int fd,
+  unsigned int sequence,
+  unsigned int tv_sec,
+  unsigned int tv_usec,
+  unsigned int crtc_id,
+  void *user_data);
+
 } drmEventContext, *drmEventContextPtr;

 extern int drmHandleEvent(int fd, drmEventContextPtr evctx);
diff --git a/xf86drmMode.c b/xf86drmMode.c
index fc19504..a9119a2 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -879,7 +879,8 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx)
int len, i;
struct drm_event *e;
struct drm_event_vblank *vblank;
-   
+   void *user_data;
+
/* The DRM read semantics guarantees that we always get only
 * complete events. */

@@ -905,15 +906,22 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx)
  U642VOID (vblank->user_data));
break;
case DRM_EVENT_FLIP_COMPLETE:
-   if (evctx->version < 2 ||
-   evctx->page_flip_handler == NULL)
-   break;
vblank = (struct drm_event_vblank *) e;
-   evctx->page_flip_handler(fd,
-vblank->sequence,
-vblank->tv_sec,
-vblank->tv_usec,
-U642VOID (vblank->user_data));
+   user_data = U642VOID (vblank->user_data);
+
+   if (evctx->version >= 3 && evctx->page_flip_handler2)
+   evctx->page_flip_handler2(fd,
+vblank->sequence,
+vblank->tv_sec,
+vblank->tv_usec,
+vblank->crtc_id,
+user_data);
+   else if (evctx->version == 2 && 
evctx->page_flip_handler)
+   evctx->page_flip_handler(fd,
+vblank->sequence,
+vblank->tv_sec,
+vblank->tv_usec,
+user_data);
break;
default:
break;
-- 
2.4.3

[PATCH] drm: Repurpose reserved field of drm_event_vlank to crtc_id

[Ander Conselvan de Oliveira]

With the atomic API, it is possible that a single commit affects
multiple crtcs. If the user requests an event with that commit, one
event will be sent for each CRTC, but it is not possible to distinguish
which crtc an event is for in user space. To solve this, the reserved
field in struct drm_vblank_event is repurposed to include the crtc_id
which the event is for.

The DRM_CAP_CRTC_IN_VBLANK_EVENT is added to allow userspace to query if
the crtc field will be set properly.

Signed-off-by: Ander Conselvan de Oliveira 
---
 drivers/gpu/drm/drm_atomic.c | 7 +--
 drivers/gpu/drm/drm_crtc.c   | 1 +
 drivers/gpu/drm/drm_ioctl.c  | 3 +++
 include/uapi/drm/drm.h   | 3 ++-
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1066e4b..2a76d10 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1304,7 +1304,8 @@ EXPORT_SYMBOL(drm_atomic_async_commit);
  */

 static struct drm_pending_vblank_event *create_vblank_event(
-   struct drm_device *dev, struct drm_file *file_priv, uint64_t 
user_data)
+   struct drm_device *dev, struct drm_file *file_priv,
+   uint64_t user_data, struct drm_crtc *crtc)
 {
struct drm_pending_vblank_event *e = NULL;
unsigned long flags;
@@ -1328,6 +1329,7 @@ static struct drm_pending_vblank_event 
*create_vblank_event(
e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
e->event.base.length = sizeof e->event;
e->event.user_data = user_data;
+   e->event.crtc_id = crtc->base.id;
e->base.event = >event.base;
e->base.file_priv = file_priv;
e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
@@ -1529,7 +1531,8 @@ retry:
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct drm_pending_vblank_event *e;

-   e = create_vblank_event(dev, file_priv, arg->user_data);
+   e = create_vblank_event(dev, file_priv, arg->user_data,
+   crtc);
if (!e) {
ret = -ENOMEM;
goto out;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 33d877c..98fe624 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5212,6 +5212,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
e->event.base.length = sizeof(e->event);
e->event.user_data = page_flip->user_data;
+   e->event.crtc_id = crtc->base.id;
e->base.event = >event.base;
e->base.file_priv = file_priv;
e->base.destroy =
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index b1d303f..152aeba 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -312,6 +312,9 @@ static int drm_getcap(struct drm_device *dev, void *data, 
struct drm_file *file_
case DRM_CAP_ADDFB2_MODIFIERS:
req->value = dev->mode_config.allow_fb_modifiers;
break;
+   case DRM_CAP_CRTC_IN_VBLANK_EVENT:
+   req->value = 1;
+   break;
default:
return -EINVAL;
}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 3801584..ecd9e96 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -631,6 +631,7 @@ struct drm_gem_open {
 #define DRM_CAP_CURSOR_WIDTH   0x8
 #define DRM_CAP_CURSOR_HEIGHT  0x9
 #define DRM_CAP_ADDFB2_MODIFIERS   0x10
+#define DRM_CAP_CRTC_IN_VBLANK_EVENT   0x11

 /** DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
@@ -826,7 +827,7 @@ struct drm_event_vblank {
__u32 tv_sec;
__u32 tv_usec;
__u32 sequence;
-   __u32 reserved;
+   __u32 crtc_id;
 };

 /* typedef area */
-- 
2.4.3

[RFC] Repurpose reserved field in vblank event to crtc_id

[Ander Conselvan de Oliveira]

Hi,

Right now, if an atomic commit sends multiple vblank events it is not
possible to distiguish for each crtc each event is for in userspace.
This series changes that be repurposing the reserved field in struct
drm_event_vblank.

Would this be a sane thing to do?

Thanks,
Ander

 drivers/gpu/drm/drm_atomic.c | 7 +--
 drivers/gpu/drm/drm_crtc.c   | 1 +
 drivers/gpu/drm/drm_ioctl.c  | 3 +++
 include/uapi/drm/drm.h   | 3 ++-
 4 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.4.3

[Intel-gfx] [PATCH 4/4] drm/atomic-helpers: Make encoder picking more robust

[Ander Conselvan De Oliveira]

For the series:

Reviewed-by: Ander Conselvan de Oliveira 


On Mon, 2015-08-03 at 17:24 +0200, Daniel Vetter wrote:
> We've had a few issues with atomic where subtle bugs in the encoder
> picking logic lead to accidental self-stealing of the encoder,
> resulting in a NULL connector_state->crtc in update_connector_routing
> and subsequent.
> 
> Linus applied some duct-tape for an mst regression in
> 
> commit 27667f4744fc5a0f3e50910e78740bac5670d18b
> Author: Linus Torvalds 
> Date:   Wed Jul 29 22:18:16 2015 -0700
> 
> i915: temporary fix for DP MST docking station NULL pointer dereference
> 
> But that was incomplete (the code will still oops when debuggin is
> enabled) and mangled the state even further. So instead WARN and bail
> out as the more future-proof option.
> 
> Cc: Theodore Ts'o 
> Cc: Linus Torvalds 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 8694ca9beee3..9dcc7280e572 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -234,13 +234,14 @@ update_connector_routing(struct drm_atomic_state 
> *state, int conn_idx)
>   }
>   }
>  
> + if (WARN_ON(!connector_state->crtc))
> + return -EINVAL;
> +
>   connector_state->best_encoder = new_encoder;
> - if (connector_state->crtc) {
> - idx = drm_crtc_index(connector_state->crtc);
> + idx = drm_crtc_index(connector_state->crtc);
>  
> - crtc_state = state->crtc_states[idx];
> - crtc_state->mode_changed = true;
> - }
> + crtc_state = state->crtc_states[idx];
> + crtc_state->mode_changed = true;
>  
>   DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on 
> [CRTC:%d]\n",
>connector->base.id,

[PATCH 2/6] drm/atomic: pass old crtc state to atomic_begin/flush.

[Ander Conselvan De Oliveira]

Reviewed-by: Ander Conselvan de Oliveira 

On Tue, 2015-07-21 at 13:28 +0200, Maarten Lankhorst wrote:
> In intel it's useful to keep track of some state changes with old
> crtc state vs new state, for example to disable initial planes or
> when a modeset's prevented during fastboot.
> 
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c |  6 --
>  drivers/gpu/drm/drm_atomic_helper.c|  8 
>  drivers/gpu/drm/drm_plane_helper.c |  4 ++--
>  drivers/gpu/drm/i915/intel_display.c   | 10 ++
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c   |  6 --
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c   |  6 --
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c |  6 --
>  drivers/gpu/drm/sti/sti_drm_crtc.c |  6 --
>  drivers/gpu/drm/tegra/dc.c |  6 --
>  include/drm/drm_crtc_helper.h  |  6 --
>  10 files changed, 40 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index f69b92535505..8b8fe3762ca9 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -239,7 +239,8 @@ static int atmel_hlcdc_crtc_atomic_check(struct 
> drm_crtc *c,
>   return atmel_hlcdc_plane_prepare_disc_area(s);
>  }
>  
> -static void atmel_hlcdc_crtc_atomic_begin(struct drm_crtc *c)
> +static void atmel_hlcdc_crtc_atomic_begin(struct drm_crtc *c,
> +   struct drm_crtc_state 
> *old_s)
>  {
>   struct atmel_hlcdc_crtc *crtc = 
> drm_crtc_to_atmel_hlcdc_crtc(c);
>  
> @@ -253,7 +254,8 @@ static void atmel_hlcdc_crtc_atomic_begin(struct 
> drm_crtc *c)
>   }
>  }
>  
> -static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc)
> +static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc,
> +   struct drm_crtc_state 
> *old_s)
>  {
>   /* TODO: write common plane control register if available */
>  }
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index ac6601071414..4a48d76c4fb1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1170,7 +1170,7 @@ void drm_atomic_helper_commit_planes(struct 
> drm_device *dev,
>   if (!funcs || !funcs->atomic_begin)
>   continue;
>  
> - funcs->atomic_begin(crtc);
> + funcs->atomic_begin(crtc, old_crtc_state);
>   }
>  
>   for_each_plane_in_state(old_state, plane, old_plane_state, 
> i) {
> @@ -1200,7 +1200,7 @@ void drm_atomic_helper_commit_planes(struct 
> drm_device *dev,
>   if (!funcs || !funcs->atomic_flush)
>   continue;
>  
> - funcs->atomic_flush(crtc);
> + funcs->atomic_flush(crtc, old_crtc_state);
>   }
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
> @@ -1236,7 +1236,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct 
> drm_crtc_state *old_crtc_state)
>  
>   crtc_funcs = crtc->helper_private;
>   if (crtc_funcs && crtc_funcs->atomic_begin)
> - crtc_funcs->atomic_begin(crtc);
> + crtc_funcs->atomic_begin(crtc, old_crtc_state);
>  
>   drm_for_each_plane_mask(plane, crtc->dev, plane_mask) {
>   struct drm_plane_state *old_plane_state =
> @@ -1259,7 +1259,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct 
> drm_crtc_state *old_crtc_state)
>   }
>  
>   if (crtc_funcs && crtc_funcs->atomic_flush)
> - crtc_funcs->atomic_flush(crtc);
> + crtc_funcs->atomic_flush(crtc, old_crtc_state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_commit_planes_on_crtc);
>  
> diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> b/drivers/gpu/drm/drm_plane_helper.c
> index 03b7afe455e6..cb5dab4c4337 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -436,7 +436,7 @@ int drm_plane_helper_commit(struct drm_plane 
> *plane,
>  
>   for (i = 0; i < 2; i++) {
>   if (crtc_funcs[i] && crtc_funcs[i]->atomic_begin)
> - crtc_funcs[i]->atomic_begin(crtc[i]);
> + crtc_funcs[i]->atomic_begin(crtc[i], crtc[i]
> ->state);
>   }
>  
>   /*
> @@ -451,7 +451,7 @@ int drm_plane_helper_commit(struct drm_plane 
> *plane,
>  
>   for (i = 0; i < 2; 

[PATCH 1/6] drm/atomic: add connectors_changed to separate it from mode_changed, v2

[Ander Conselvan De Oliveira]

On Tue, 2015-07-21 at 13:28 +0200, Maarten Lankhorst wrote:
> This can be a separate case from mode_changed, when connectors stay 
> the
> same but only the mode is different. Drivers may choose to implement 
> specific
> optimizations to prevent a full modeset for this case.
> 
> Changes since v1:
> - Update kerneldocs slightly.
> 
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: Maarten Lankhorst 

I have a couple of nits below, but anyway,

Reviewed-by: Ander Conselvan de Oliveira 

> ---
>  drivers/gpu/drm/drm_atomic_helper.c  | 39 
> +++-
>  drivers/gpu/drm/i915/intel_display.c |  2 +-
>  include/drm/drm_atomic.h |  3 ++-
>  include/drm/drm_crtc.h   |  8 +---
>  4 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 0ea8c5d476ef..ac6601071414 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -124,7 +124,7 @@ steal_encoder(struct drm_atomic_state *state,
>   if (IS_ERR(crtc_state))
>   return PTR_ERR(crtc_state);
>  
> - crtc_state->mode_changed = true;
> + crtc_state->connectors_changed = true;
>  
>   list_for_each_entry(connector, >connector_list, 
> head) {
>   if (connector->state->best_encoder != encoder)
> @@ -174,14 +174,14 @@ update_connector_routing(struct 
> drm_atomic_state *state, int conn_idx)
>   idx = drm_crtc_index(connector->state
> ->crtc);
>  
>   crtc_state = state->crtc_states[idx];
> - crtc_state->mode_changed = true;
> + crtc_state->connectors_changed = true;
>   }
>  
>   if (connector_state->crtc) {
>   idx = drm_crtc_index(connector_state->crtc);
>  
>   crtc_state = state->crtc_states[idx];
> - crtc_state->mode_changed = true;
> + crtc_state->connectors_changed = true;
>   }
>   }
>  
> @@ -233,7 +233,7 @@ update_connector_routing(struct drm_atomic_state 
> *state, int conn_idx)
>   idx = drm_crtc_index(connector_state->crtc);
>  
>   crtc_state = state->crtc_states[idx];
> - crtc_state->mode_changed = true;
> + crtc_state->connectors_changed = true;
>  

There's a comment above the call to update_connector_routing() that
mentions setting crtc_state->mode_changed. That should be updated to
reflect the changes above.

>   DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on 
> [CRTC:%d]\n",
>connector->base.id,
> 

[...]

> @@ -338,9 +340,14 @@ mode_fixup(struct drm_atomic_state *state)
>   *
>   * Check the state object to see if the requested state is physically 
> possible.
>   * This does all the crtc and connector related computations for an atomic
> - * update. It computes and updates crtc_state->mode_changed, adds any 
> additional
> - * connectors needed for full modesets and calls down into ->mode_fixup
> - * functions of the driver backend.
> + * update and adds any additional connectors needed for full modesets and 
> calls

The first 'and' should be replaced with a comma.

[...]

Ander

[Intel-gfx] [PATCH] drm/i915: Clear pipe's pll hw state in hsw_dp_set_ddi_pll_sel()

[Ander Conselvan De Oliveira]

On Tue, 2015-06-30 at 18:41 +0300, Jani Nikula wrote:
> On Tue, 30 Jun 2015, Daniel Vetter  wrote:
> > On Tue, Jun 30, 2015 at 04:47:06PM +0300, Jani Nikula wrote:
> >> On Tue, 30 Jun 2015, Ander Conselvan de Oliveira 
> >>  wrote:
> >> > Similarly to what is done for SKL, clear the dpll_hw_state of the pipe
> >> > config in hsw_dp_set_ddi_pll_sel(), since it main contain stale values.
> >> > That can happen if a crtc that was previously driving an HDMI connector
> >> > switches to a DP connector. In that case, the wrpll field was left with
> >> > its old value, leading to warnings like the one below:
> >> >
> >> > [drm:check_crtc_state [i915]] *ERROR* mismatch in dpll_hw_state.wrpll 
> >> > (expected 0xb035061f, found 0x)
> >> > [ cut here ]
> >> > WARNING: CPU: 1 PID: 767 at drivers/gpu/drm/i915/intel_display.c:12324 
> >> > check_crtc_state+0x975/0x10b0 [i915]()
> >> > pipe state doesn't match!
> >> >
> >> > This regression was indroduced in
> >> >
> >> > commit dd3cd74acf12723045a64f1f2c6298ac7b34a5d5
> >> > Author: Ander Conselvan de Oliveira  >> > intel.com>
> >> > Date:   Fri May 15 13:34:29 2015 +0300
> >> >
> >> > drm/i915: Don't overwrite (e)DP PLL selection on SKL
> >> >
> >> > Signed-off-by: Ander Conselvan de Oliveira  >> > at intel.com>
> >> 
> >> Reported-by: Linus Torvalds 
> >> Tested-by: Jani Nikula 
> >
> > Yeah makes sense as a fix for 4.2. But for 4.3 I wonder whether the
> > original commit that started this chain needs to be changed a bit:
> >
> > commit 4978cc93d9ac240b435ce60431aef24239b4c270
> > Author: Ander Conselvan de Oliveira  > intel.com>
> > Date:   Tue Apr 21 17:13:21 2015 +0300
> >
> > drm/i915: Preserve shared DPLL information in new pipe_config
> >
> > All the trouble this caused is because it not only preserves the sharing
> > config (in crtc_state->shared_dpll) but also the ->dpll_hw_state. And I
> > think with Maarten's latest code (for 4.3) we'd just do an unconditional
> > compute_config (need it for fast pfit updates and fastboot), which means
> > the bogus values in ->dpll_hw_state aren't a problem any more since we'll
> > overwrite them again. And then we could remove that sprinkle of memsets we
> > have all over, which would be good (since the current approach is
> > obviously a bit fragile). Anyway:
> >
> > Reviewed-by: Daniel Vetter 
> 
> Pushed to drm-intel-next-fixes, thanks for the patch and review. One
> down, another one left to fix.

I made some progress on the second issue, but I'm afraid Jani might have
a found a third bug. The warning he gets happens because we try to wait
for vblanks while updating the primary plane during the modeset. At that
point, the crtc is off. The problem is in intel_check_primary_plane(),
which is called from drm_atomic_helper_check_planes(). That function
makes decisions about waiting for a vblank based on intel_crtc->active.
Since the check is called before we disable the crtcs, active might be
true, even though the plane update is done with crtcs disable.

The patch below makes the warning go away, but I still need to figure
out how to set crtc_state->planes_changed properly if we are going down
that route.

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index dcb1d25..f14727c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12480,10 +12480,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc,

intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]");

-   ret = drm_atomic_helper_check_planes(state->dev, state);
-   if (ret)
-   return ERR_PTR(ret);
-
return pipe_config;
 }


The backtrace on Linus' machine is different, though. It comes from the
call to intel_crtc_disable_planes() in __intel_set_mode(). That would
indicate we have a crtc with crtc->state->enable == true but that is
actually inactive. I'm still not sure how we can get in that state.

Ander

[PATCH] drm/i915: Clear pipe's pll hw state in hsw_dp_set_ddi_pll_sel()

[Ander Conselvan de Oliveira]

Similarly to what is done for SKL, clear the dpll_hw_state of the pipe
config in hsw_dp_set_ddi_pll_sel(), since it main contain stale values.
That can happen if a crtc that was previously driving an HDMI connector
switches to a DP connector. In that case, the wrpll field was left with
its old value, leading to warnings like the one below:

[drm:check_crtc_state [i915]] *ERROR* mismatch in dpll_hw_state.wrpll (expected 
0xb035061f, found 0x)
[ cut here ]
WARNING: CPU: 1 PID: 767 at drivers/gpu/drm/i915/intel_display.c:12324 
check_crtc_state+0x975/0x10b0 [i915]()
pipe state doesn't match!

This regression was indroduced in

commit dd3cd74acf12723045a64f1f2c6298ac7b34a5d5
Author: Ander Conselvan de Oliveira 
Date:   Fri May 15 13:34:29 2015 +0300

drm/i915: Don't overwrite (e)DP PLL selection on SKL

Signed-off-by: Ander Conselvan de Oliveira 
---

Only compile tested, for the wrpll warning.

Thanks,
Ander

 drivers/gpu/drm/i915/intel_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4ebfc3a..fbd9ac3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1147,6 +1147,9 @@ skl_edp_set_pll_config(struct intel_crtc_state 
*pipe_config, int link_clock)
 static void
 hsw_dp_set_ddi_pll_sel(struct intel_crtc_state *pipe_config, int link_bw)
 {
+   memset(_config->dpll_hw_state, 0,
+  sizeof(pipe_config->dpll_hw_state));
+
switch (link_bw) {
case DP_LINK_BW_1_62:
pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
-- 
2.1.0

[git pull] drm tree for 4.2

[Ander Conselvan De Oliveira]

On Fri, 2015-06-26 at 14:43 -0700, Linus Torvalds wrote:
> On Thu, Jun 25, 2015 at 6:00 PM, Dave Airlie  wrote:
> >
> > This is the main drm pull request for v4.2.
> 
> It seems to work ok for me, but it causes quite a few new warnings on
> my Sony VAIO Pro laptop. It's (once more) a regular i5-4200U CPU (aka
> Haswell, aka 4th gen Intel Core i5)
> 
> Most of them are in check_crtc_state(), and I currently have 18 of
> these in my log:
> 
>   [drm:check_crtc_state [i915]] *ERROR* mismatch in
> dpll_hw_state.wrpll (expected 0x90280202, found 0x)
>   WARNING: CPU: 0 PID: 115 at
> drivers/gpu/drm/i915/intel_display.c:12319
> check_crtc_state+0x8be/0xf60 [i915]()
>   pipe state doesn't match!
> 
> but there's a few others too:
> 
>   WARNING: CPU: 3 PID: 1871 at
> drivers/gpu/drm/i915/intel_display.c:1362 hsw_disable_ips+0x34/0x160
> [i915]()
>   plane A assertion failure (expected on, current off)
> 
>   WARNING: CPU: 3 PID: 1871 at drivers/gpu/drm/drm_irq.c:1162
> drm_wait_one_vblank+0x148/0x1a0 [drm]()
>   vblank not available on crtc 0, ret=-22
> 
> and the backtraces aren't all that interesting, but I'm attaching the
> cleaned-up dmesg, duplicate callchains and all.

Please provide a full dmesg with drm.debug=0x1f in the kernel command
line.

Thanks,
Ander

[PULL] drm-intel-next-fixes

[Ander Conselvan De Oliveira]

There is

commit bd4b4827acdc00bf9e71f939d160102021d10d4f
Author: Ander Conselvan de Oliveira

Date:   Fri May 29 14:28:09 2015 +0300

drm/i915: Silence compiler warning

in -nightly to fix that same issue. I didn't realize this was also
needed in -next-fixes.


Ander

On Fri, 2015-06-19 at 17:24 +0200, Daniel Vetter wrote:
> On Fri, Jun 19, 2015 at 01:48:13PM +1000, Dave Airlie wrote:
> > On 18 June 2015 at 16:04, Jani Nikula  wrote:
> > >
> > > Hi Dave, i915 fixes for drm-next/v4.2.
> > >
> > > BR,
> > > Jani.
> > 
> > And my gcc says:
> > 
> > /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i915/intel_display.c:
> > In function ‘__intel_set_mode’:
> > /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i915/intel_display.c:11850:14:
> > warning: ‘crtc_state’ may be used uninitialized in this function
> > [-Wmaybe-uninitialized]
> >   return state->mode_changed || state->active_changed;
> >   ^
> > /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i915/intel_display.c:11860:25:
> > note: ‘crtc_state’ was declared here
> >   struct drm_crtc_state *crtc_state;
> >  ^
> > /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i915/intel_display.c:11874:6:
> > warning: ‘crtc’ may be used uninitialized in this function
> > [-Wmaybe-uninitialized]
> >if (crtc != intel_encoder->base.crtc)
> >   ^
> > /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i915/intel_display.c:11859:19:
> > note: ‘crtc’ was declared here
> >   struct drm_crtc *crtc;
> >^
> > 
> > No idea if this is true, but I don't think I've seen it before now.
> > 
> > gcc 5.1.1 on fedora 22
> 
> Yeah this is new with Ander's patches. gcc Doesn't know that we have at
> least 1 crtc and hence crtc are guaranteed to be initiliazed. I
> think you should be able to shut it up with
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index e047105837c9..5ade250dc6d7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11856,8 +11856,8 @@ intel_modeset_update_state(struct drm_atomic_state 
> *state)
>   struct drm_device *dev = state->dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct intel_encoder *intel_encoder;
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *crtc_state;
> + struct drm_crtc *crtc = NULL;
> + struct drm_crtc_state *crtc_state = NULL;
>   struct drm_connector *connector;
>   int i;
>  
> But the entire Finland team is out of office (celebrating solstice), so
> might be better to wait for Monday for them to confirm. Otherwise just
> apply this fixup with my ack if you want to send out the merge window pull
> asap.
> 
> Cheers, Daniel

[Fwd: Re: [PATCH] drm/i915: Properly initialize SDVO analog connectors]

[Ander Conselvan De Oliveira]

Thanks for testing.

 Forwarded Message 
From: Stefan Lippers-Hollmann <s@gmx.de>
To: Ander Conselvan de Oliveira 
Subject: Re: [PATCH] drm/i915: Properly initialize SDVO analog
connectors
Date: Mon, 8 Jun 2015 12:16:04 +0200

Hi

On 2015-06-08, Ander Conselvan de Oliveira wrote:
> In the commit below, I missed the connector allocation in the function
> intel_sdvo_analog_init(), leading to those connectors to have a NULL
> state pointer.

Thanks a lot, this patch is working (on top of 4.1-rc7).

> commit 08d9bc920d465d762cac9383249c19bf69a2
> Author: Ander Conselvan de Oliveira

> Date:   Fri Apr 10 10:59:10 2015 +0300
> 
> drm/i915: Allocate connector state together with the connectors
> 
> Signed-off-by: Ander Conselvan de Oliveira


Feel free to add:

Tested-by: Stefan Lippers-Hollmann 

Regards
Stefan Lippers-Hollmann

git pull] drm for v4.1-rc1

[Ander Conselvan De Oliveira]

On Mon, 2015-06-08 at 11:06 +0300, Ander Conselvan De Oliveira wrote:
> On Sun, 2015-06-07 at 04:32 +0200, Stefan Lippers-Hollmann wrote:
> > Hi
> > 
> > On 2015-06-07, Ville Syrjälä wrote:
> > > On Fri, Jun 05, 2015 at 11:18:21PM +0200, Stefan Lippers-Hollmann wrote:
> > > > Hi
> > > > 
> > > > On 2015-04-20, Dave Airlie wrote:
> > > > [...]
> > > > > The following changes since commit 
> > > > > 09d51602cf84a1264946711dd4ea0dddbac599a1:
> > > > > 
> > > > >   Merge branch 'turbostat' of 
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux (2015-04-19 
> > > > > 14:31:41 -0700)
> > > > > 
> > > > > are available in the git repository at:
> > > > > 
> > > > >   git://people.freedesktop.org/~airlied/linux drm-next-merged
> > > > > 
> > > > > for you to fetch changes up to 
> > > > > 2c33ce009ca2389dbf0535d0672214d09738e35e:
> > > > > 
> > > > >   Merge Linus master into drm-next (2015-04-20 13:05:20 +1000)
> > > > [...]
> > > > > Ander Conselvan de Oliveira (28):
> > > > [...]
> > > > >   drm/i915: Allocate connector state together with the connectors
> > > > [...]
> > > > 
> > > > This commit introduces a regression relative to v4.0 on an Intel 
> > > > D945GCLF2 mainboard[1] (Atom 330) with Intel 82945G/GZ onboard graphics 
> > > > using its (only-) VGA connector for me.
> > > > 
> > > > v4.1-rc6-52-gff25ea8:
> > > > [   13.265699] BUG: unable to handle kernel NULL pointer dereference at 
> > > > 0010
> > > > [   13.265723] IP: [] 
> > > > intel_modeset_update_connector_atomic_state+0x61/0x90 [i915]
> > > 
> > > Hmm. Smells like a connector with a NULL state pointer, and the bad
> > > commit touched exactly the part that sets it up. I can't immediately
> > > spot any place where we'd forget to set it up though.
> > > 
> > > Can you try with something like this so we'd at least find out which
> > > connector(s) is/are at fault here?
> > 
> > With the patch applied, the kernel (v4.1-rc6-104-g4b17069) locks up even
> > harder, so I had to switch to a serial console in order to fetch the 
> > boot messages:
> > 
> > [   13.492784] connector = 880079bb8000
> > [   13.910439] connector = 8800795b5800
> > [   14.463114] connector = 8800795b6000
> > [   14.700707] connector = 8800795b6800
> > [   14.869418] connector = 8800795b7000
> > [   14.923848] connector = 8800795b7000
> > 
> > Full, gzipped, bootlog attached - thanks a lot for your efforts.
> 
> Could you repeat the process with drm.debug=0xe in your kernel command
> line and send the logs again?

Never mind, Ville's patch produced all the information necessary. Please
give the patch I just sent a try.

Thanks,
Ander

[PATCH] drm/i915: Properly initialize SDVO analog connectors

[Ander Conselvan de Oliveira]

In the commit below, I missed the connector allocation in the function
intel_sdvo_analog_init(), leading to those connectors to have a NULL
state pointer.

commit 08d9bc920d465d762cac9383249c19bf69a2
Author: Ander Conselvan de Oliveira 
Date:   Fri Apr 10 10:59:10 2015 +0300

drm/i915: Allocate connector state together with the connectors

Signed-off-by: Ander Conselvan de Oliveira 
---
 drivers/gpu/drm/i915/intel_sdvo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c 
b/drivers/gpu/drm/i915/intel_sdvo.c
index d24ef75..aa2fd75 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2554,7 +2554,7 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int 
device)

DRM_DEBUG_KMS("initialising analog device %d\n", device);

-   intel_sdvo_connector = kzalloc(sizeof(*intel_sdvo_connector), 
GFP_KERNEL);
+   intel_sdvo_connector = intel_sdvo_connector_alloc();
if (!intel_sdvo_connector)
return false;

-- 
2.1.0

git pull] drm for v4.1-rc1

[Ander Conselvan De Oliveira]

On Sun, 2015-06-07 at 04:32 +0200, Stefan Lippers-Hollmann wrote:
> Hi
> 
> On 2015-06-07, Ville Syrjälä wrote:
> > On Fri, Jun 05, 2015 at 11:18:21PM +0200, Stefan Lippers-Hollmann wrote:
> > > Hi
> > > 
> > > On 2015-04-20, Dave Airlie wrote:
> > > [...]
> > > > The following changes since commit 
> > > > 09d51602cf84a1264946711dd4ea0dddbac599a1:
> > > > 
> > > >   Merge branch 'turbostat' of 
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux (2015-04-19 
> > > > 14:31:41 -0700)
> > > > 
> > > > are available in the git repository at:
> > > > 
> > > >   git://people.freedesktop.org/~airlied/linux drm-next-merged
> > > > 
> > > > for you to fetch changes up to 2c33ce009ca2389dbf0535d0672214d09738e35e:
> > > > 
> > > >   Merge Linus master into drm-next (2015-04-20 13:05:20 +1000)
> > > [...]
> > > > Ander Conselvan de Oliveira (28):
> > > [...]
> > > >   drm/i915: Allocate connector state together with the connectors
> > > [...]
> > > 
> > > This commit introduces a regression relative to v4.0 on an Intel 
> > > D945GCLF2 mainboard[1] (Atom 330) with Intel 82945G/GZ onboard graphics 
> > > using its (only-) VGA connector for me.
> > > 
> > > v4.1-rc6-52-gff25ea8:
> > > [   13.265699] BUG: unable to handle kernel NULL pointer dereference at 
> > > 0010
> > > [   13.265723] IP: [] 
> > > intel_modeset_update_connector_atomic_state+0x61/0x90 [i915]
> > 
> > Hmm. Smells like a connector with a NULL state pointer, and the bad
> > commit touched exactly the part that sets it up. I can't immediately
> > spot any place where we'd forget to set it up though.
> > 
> > Can you try with something like this so we'd at least find out which
> > connector(s) is/are at fault here?
> 
> With the patch applied, the kernel (v4.1-rc6-104-g4b17069) locks up even
> harder, so I had to switch to a serial console in order to fetch the 
> boot messages:
> 
> [   13.492784] connector = 880079bb8000
> [   13.910439] connector = 8800795b5800
> [   14.463114] connector = 8800795b6000
> [   14.700707] connector = 8800795b6800
> [   14.869418] connector = 8800795b7000
> [   14.923848] connector = 8800795b7000
> 
> Full, gzipped, bootlog attached - thanks a lot for your efforts.

Could you repeat the process with drm.debug=0xe in your kernel command
line and send the logs again?

Thanks,
Ander

[PATCH] drm/atomic: Don't try to free a NULL state

[Ander Conselvan de Oliveira]

Consistently with other free functions, handle the NULL case without
oopsing.

Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Ander Conselvan de Oliveira 
---
 drivers/gpu/drm/drm_atomic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 23034e8..88b2790 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -179,6 +179,9 @@ EXPORT_SYMBOL(drm_atomic_state_clear);
  */
 void drm_atomic_state_free(struct drm_atomic_state *state)
 {
+   if (!state)
+   return;
+
drm_atomic_state_clear(state);

DRM_DEBUG_ATOMIC("Freeing atomic state %p\n", state);
-- 
2.1.0

[PATCH] drm/atomic: Clear crtcs, connectors and planes when clearing state

[Ander Conselvan de Oliveira]

Users of the atomic state assume that if the pointer to a crtc, plane or
connector is not NULL in the respective object vector, than the state
for that object in *_states vector also won't be NULL. That assumption
was broken by drm_atomic_state_clear(), which would clear the state
pointer but leave the pointer to the object still set.

This fixes a NULL pointer dereference in i915 caused by the use of
drm_atomic_state_clear().

Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Ander Conselvan de Oliveira 
---
 drivers/gpu/drm/drm_atomic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5d3abe3..00ea881 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -134,6 +134,7 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)

connector->funcs->atomic_destroy_state(connector,
   
state->connector_states[i]);
+   state->connectors[i] = NULL;
state->connector_states[i] = NULL;
}

@@ -145,6 +146,7 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)

crtc->funcs->atomic_destroy_state(crtc,
  state->crtc_states[i]);
+   state->crtcs[i] = NULL;
state->crtc_states[i] = NULL;
}

@@ -156,6 +158,7 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)

plane->funcs->atomic_destroy_state(plane,
   state->plane_states[i]);
+   state->planes[i] = NULL;
state->plane_states[i] = NULL;
}
 }
-- 
2.1.0

[PATCH] drm/atomic: Fix potential use of state after free

[Ander Conselvan de Oliveira]

The atomic helpers rely on drm_atomic_state_clear() to reset an atomic
state if a retry is needed due to the w/w mutexes. The subsequent calls
to drm_atomic_get_{crtc,plane,...}_state() would then return the stale
pointers in state->{crtc,plane,...}_states.

Signed-off-by: Ander Conselvan de Oliveira 
---
 drivers/gpu/drm/drm_atomic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1b31982..9d16fa4 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -134,6 +134,7 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)

connector->funcs->atomic_destroy_state(connector,
   
state->connector_states[i]);
+   state->connector_states[i] = NULL;
}

for (i = 0; i < config->num_crtc; i++) {
@@ -144,6 +145,7 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)

crtc->funcs->atomic_destroy_state(crtc,
  state->crtc_states[i]);
+   state->crtc_states[i] = NULL;
}

for (i = 0; i < config->num_total_plane; i++) {
@@ -154,6 +156,7 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)

plane->funcs->atomic_destroy_state(plane,
   state->plane_states[i]);
+   state->plane_states[i] = NULL;
}
 }
 EXPORT_SYMBOL(drm_atomic_state_clear);
-- 
1.9.1

[Intel-gfx] [PATCH 1/9] drm: add helper to get crtc timings (v4)

[Ander Conselvan de Oliveira]

On 11/24/2014 09:52 PM, Matt Roper wrote:
> From: Gustavo Padovan 
>
> We need to get hdisplay and vdisplay in a few places so create a
> helper to make our job easier.
>
> v2 (by Matt): Use new stereo doubling function (suggested by Ville)
>
> v3 (by Matt):
>   - Add missing kerneldoc (Daniel)
>   - Use drm_mode_copy() (Jani)
>
> v4 (by Matt):
>   - Drop stereo doubling function again; add 'stereo only' flag
> to drm_mode_set_crtcinfo() instead (Ville)
>
> Cc: dri-devel at lists.freedesktop.org
> Suggested-by: Ville Syrjälä 
> Signed-off-by: Gustavo Padovan 
> Signed-off-by: Matt Roper 
> ---
>   drivers/gpu/drm/drm_crtc.c   | 32 ++--
>   drivers/gpu/drm/drm_modes.c  | 24 ++--
>   drivers/gpu/drm/i915/intel_display.c |  6 +++---
>   include/drm/drm_crtc.h   |  2 ++
>   include/drm/drm_modes.h  |  3 +++
>   5 files changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 589a921..f06f1b4 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2499,6 +2499,27 @@ int drm_mode_set_config_internal(struct drm_mode_set 
> *set)
>   EXPORT_SYMBOL(drm_mode_set_config_internal);
>
>   /**
> + * drm_crtc_get_hv_timing - Fetches hdisplay/vdisplay for given mode
> + * @mode: mode to query
> + * @hdisplay: hdisplay value to fill in
> + * @vdisplay: vdisplay value to fill in
> + *
> + * The vdisplay value will be doubled if the specified mode is a stereo mode 
> of
> + * the appropriate layout.
> + */
> +void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
> + int *hdisplay, int *vdisplay)
> +{
> + struct drm_display_mode adjusted;
> +
> + drm_mode_copy(, mode);
> + drm_mode_set_crtcinfo(, CRTC_STEREO_DOUBLE_ONLY);
> + *hdisplay = adjusted.crtc_hdisplay;
> + *vdisplay = adjusted.crtc_vdisplay;
> +}
> +EXPORT_SYMBOL(drm_crtc_get_hv_timing);
> +
> +/**
>* drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
>* CRTC viewport
>* @crtc: CRTC that framebuffer will be displayed on
> @@ -2515,16 +2536,7 @@ int drm_crtc_check_viewport(const struct drm_crtc 
> *crtc,
>   {
>   int hdisplay, vdisplay;
>
> - hdisplay = mode->hdisplay;
> - vdisplay = mode->vdisplay;
> -
> - if (drm_mode_is_stereo(mode)) {
> - struct drm_display_mode adjusted = *mode;
> -
> - drm_mode_set_crtcinfo(, CRTC_STEREO_DOUBLE);
> - hdisplay = adjusted.crtc_hdisplay;
> - vdisplay = adjusted.crtc_vdisplay;
> - }
> + drm_crtc_get_hv_timing(mode, , );

This changes the behavior slightly since before, for stereo modes, 
dbl_scan and vscan would affect hdisplay and vdisplay. If I understand 
correctly the old behavior is a bug, but it would be good to state that 
the change is intentional.

>
>   if (crtc->invert_dimensions)
>   swap(hdisplay, vdisplay);
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 6d8b941..fd5479b 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -765,18 +765,22 @@ void drm_mode_set_crtcinfo(struct drm_display_mode *p, 
> int adjust_flags)
>   }
>   }
>
> - if (p->flags & DRM_MODE_FLAG_DBLSCAN) {
> - p->crtc_vdisplay *= 2;
> - p->crtc_vsync_start *= 2;
> - p->crtc_vsync_end *= 2;
> - p->crtc_vtotal *= 2;
> + if (!(adjust_flags & CRTC_NO_DBLSCAN)) {
> + if (p->flags & DRM_MODE_FLAG_DBLSCAN) {
> + p->crtc_vdisplay *= 2;
> + p->crtc_vsync_start *= 2;
> + p->crtc_vsync_end *= 2;
> + p->crtc_vtotal *= 2;
> + }
>   }
>
> - if (p->vscan > 1) {
> - p->crtc_vdisplay *= p->vscan;
> - p->crtc_vsync_start *= p->vscan;
> - p->crtc_vsync_end *= p->vscan;
> - p->crtc_vtotal *= p->vscan;
> + if (!(adjust_flags & CRTC_NO_VSCAN)) {
> + if (p->vscan > 1) {
> + p->crtc_vdisplay *= p->vscan;
> + p->crtc_vsync_start *= p->vscan;
> + p->crtc_vsync_end *= p->vscan;
> + p->crtc_vtotal *= p->vscan;
> + }
>   }
>
>   if (adjust_flags & CRTC_STEREO_DOUBLE) {
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index a3d2a44..b87aeab 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10205,9 +10205,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>* computation to clearly distinguish it from the adjusted mode, which
>* can be changed by the connectors in the below retry loop.
>*/
> - drm_mode_set_crtcinfo(_config->requested_mode, CRTC_STEREO_DOUBLE);
> - 

[PATCH] drm: Global atomic state handling

[Ander Conselvan de Oliveira]

On 11/05/2014 12:07 AM, Daniel Vetter wrote:
>   /**
> + * struct struct drm_atomic_state - the global state object for atomic 
> updates
> + * @dev: parent DRM device
> + * @flags: state flags like async update
> + * @planes: pointer to array of plane pointers
> + * @plane_states: pointer to array of plane states pointers
> + * @crtcs: pointer to array of CRTC pointers
> + * @crtc_states: pointer to array of CRTC states pointers
> + * @connectors: pointer to array of connector pointers
> + * @connector_states: pointer to array of connector states pointers
> + * @acquire_ctx: acquire context for this atomic modeset state update
> + */
> +struct drm_atomic_state {
> +   struct drm_device *dev;
> +   uint32_t flags;
> +   struct drm_plane **planes;
> +   struct drm_plane_state **plane_states;
> +   struct drm_crtc **crtcs;
> +   struct drm_crtc_state **crtc_states;
> +   struct drm_connector **connectors;
> +   struct drm_connector_state **connector_states;
> +
> +   struct drm_modeset_acquire_ctx *acquire_ctx;
> +};


The lines above are indented with spaces instead of tabs.


Ander

[PATCH libdrm] libkms: fix the name of the intel driver in linux_sysfs_create

[Ander Conselvan de Oliveira]

Ping?

On 09/05/2012 02:30 PM, Ander Conselvan de Oliveira wrote:
> From: Ander Conselvan de Oliveira 
>
> linux_sysfs_create() checked for a driver named "intel" while the intel
> driver is called "i915". This went unnoticed because in kernels 2.6.39
> and after this code path was never reached because of the dumb buffer
> interface. On earlier kernels, kms_create() would fail.
>
> Signed-off-by: Ander Conselvan de Oliveira  intel.com>
> ---
>   libkms/linux.c |2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/libkms/linux.c b/libkms/linux.c
> index fc4f205..dde74f7 100644
> --- a/libkms/linux.c
> +++ b/libkms/linux.c
> @@ -101,7 +101,7 @@ linux_from_sysfs(int fd, struct kms_driver **out)
>   if (ret)
>   return ret;
>
> - if (!strcmp(name, "intel"))
> + if (!strcmp(name, "i915"))
>   ret = intel_create(fd, out);
>   #ifdef HAVE_VMWGFX
>   else if (!strcmp(name, "vmwgfx"))
>

Re: [PATCH libdrm] libkms: fix the name of the intel driver in linux_sysfs_create

[Ander Conselvan de Oliveira]

Ping?

On 09/05/2012 02:30 PM, Ander Conselvan de Oliveira wrote:

From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com

linux_sysfs_create() checked for a driver named intel while the intel
driver is called i915. This went unnoticed because in kernels 2.6.39
and after this code path was never reached because of the dumb buffer
interface. On earlier kernels, kms_create() would fail.

Signed-off-by: Ander Conselvan de Oliveira 
ander.conselvan.de.olive...@intel.com
---
  libkms/linux.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libkms/linux.c b/libkms/linux.c
index fc4f205..dde74f7 100644
--- a/libkms/linux.c
+++ b/libkms/linux.c
@@ -101,7 +101,7 @@ linux_from_sysfs(int fd, struct kms_driver **out)
if (ret)
return ret;

-   if (!strcmp(name, intel))
+   if (!strcmp(name, i915))
ret = intel_create(fd, out);
  #ifdef HAVE_VMWGFX
else if (!strcmp(name, vmwgfx))



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm] libkms: fix the name of the intel driver in linux_sysfs_create

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira <ander.conselvan.de.olive...@intel.com>

linux_sysfs_create() checked for a driver named "intel" while the intel
driver is called "i915". This went unnoticed because in kernels 2.6.39
and after this code path was never reached because of the dumb buffer
interface. On earlier kernels, kms_create() would fail.

Signed-off-by: Ander Conselvan de Oliveira 
---
 libkms/linux.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libkms/linux.c b/libkms/linux.c
index fc4f205..dde74f7 100644
--- a/libkms/linux.c
+++ b/libkms/linux.c
@@ -101,7 +101,7 @@ linux_from_sysfs(int fd, struct kms_driver **out)
if (ret)
return ret;

-   if (!strcmp(name, "intel"))
+   if (!strcmp(name, "i915"))
ret = intel_create(fd, out);
 #ifdef HAVE_VMWGFX
else if (!strcmp(name, "vmwgfx"))
-- 
1.7.4.1

[PATCH libdrm] libkms: fix the name of the intel driver in linux_sysfs_create

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com

linux_sysfs_create() checked for a driver named intel while the intel
driver is called i915. This went unnoticed because in kernels 2.6.39
and after this code path was never reached because of the dumb buffer
interface. On earlier kernels, kms_create() would fail.

Signed-off-by: Ander Conselvan de Oliveira 
ander.conselvan.de.olive...@intel.com
---
 libkms/linux.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libkms/linux.c b/libkms/linux.c
index fc4f205..dde74f7 100644
--- a/libkms/linux.c
+++ b/libkms/linux.c
@@ -101,7 +101,7 @@ linux_from_sysfs(int fd, struct kms_driver **out)
if (ret)
return ret;
 
-   if (!strcmp(name, intel))
+   if (!strcmp(name, i915))
ret = intel_create(fd, out);
 #ifdef HAVE_VMWGFX
else if (!strcmp(name, vmwgfx))
-- 
1.7.4.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel