Re: [PATCH] drm/i915: Add DPI FIFO empty status check

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 09 Dec 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Dec 09, 2015 at 05:05:52PM +0530, Deepak M wrote:
>> From: Gaurav K Singh <gaurav.k.singh@intel.com>
>> 
>> Before sending TURN ON packet,check the DPI
>> FIFO empty status.
>> 
>> v2: Change in commit message
>>     Checking for FIFO empty  only during TURN ON packet.
>> v3: Adding a new function for DPI FIFO empty check
>> 
>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>> Signed-off-by: Deepak M <m.deepak@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dsi.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 170ae6f..eff982b 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -60,6 +60,17 @@ static void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
>>  		DRM_ERROR("DPI FIFOs are not empty\n");
>>  }
>>  
>> +static void wait_for_dpi_fifo_empty(struct intel_dsi *intel_dsi, enum port port)
>> +{
>> +	struct drm_encoder *encoder = &intel_dsi->base.base;
>> +	struct drm_device *dev = encoder->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>
> Passing in intel_dsi just to dig out the dev_priv again seems a bit
> pointless. Could just pass dev_priv directly. But I guess it matches the
> already existing functions. 
>
> BTW for future reference, we would rather see to_i915(<whatever>)
> instead of <whatever>->dev_private.
>
> Anyway, the patch seems to do what it says. I can't really judge
> whether it's the right thing to do though.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

OTOH,

commit a799a9780eb5c874d9d7ca0bbee66401ca98c013
Author: Shobhit Kumar <shobhit.kumar@intel.com>
Date:   Thu Jul 3 16:35:40 2014 +0530

    drm/i915/vlv: DPI FIFO empty check is not needed
    
    While sending DPI SHUTDOWN command, we cannot wait for FIFO empty as
    pipes are not disabled at that time. In case of MIPI we disable port
    first and send SHUTDOWN command while pipe is still running and FIFOs
    will not be empty, causing spurious error log
    
    Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
    Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
    Signed-off-by: Jani Nikula <jani.nikula@intel.com>


>
>> +
>> +	if (wait_for((I915_READ(MIPI_GEN_FIFO_STAT(port)) & DPI_FIFO_EMPTY)
>> +					== DPI_FIFO_EMPTY, 50))
>> +		DRM_ERROR("Timeout waiting for DPI FIFO empty\n");
>> +}
>> +
>>  static void write_data(struct drm_i915_private *dev_priv, u32 reg,
>>  		       const u8 *data, u32 len)
>>  {
>> @@ -443,8 +454,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>>  			I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4);
>>  	} else {
>>  		msleep(20); /* XXX */
>> -		for_each_dsi_port(port, intel_dsi->ports)
>> +		for_each_dsi_port(port, intel_dsi->ports) {
>> +			wait_for_dpi_fifo_empty(intel_dsi, port);
>>  			dpi_send_cmd(intel_dsi, TURN_ON, false, port);
>> +		}
>>  		msleep(100);
>>  
>>  		drm_panel_enable(intel_dsi->panel);
>> -- 
>> 1.9.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx