Re: [PATCH v2 03/17] drm: Nuke mode->vrefresh

From: abhinavk@codeaurora.org
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: "Neil Armstrong" <narmstrong@baylibre.com>,
	nouveau@lists.freedesktop.org, "Guido Günther" <agx@sigxcpu.org>,
	dri-devel@lists.freedesktop.org,
	"Andrzej Hajda" <a.hajda@samsung.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	aravindh@quicinc.com, "Emil Velikov" <emil.velikov@collabora.com>,
	"Thomas Hellstrom" <thellstrom@vmware.com>,
	"Joonyoung Shim" <jy0922.shim@samsung.com>,
	"Stefan Mavrodiev" <stefan@olimex.com>,
	"Jerry Han" <hanxu5@huaqin.corp-partner.google.com>,
	"VMware Graphics" <linux-graphics-maintainer@vmware.com>,
	"Jagan Teki" <jagan@amarulasolutions.com>,
	"Robert Chiras" <robert.chiras@nxp.com>,
	pdhaval@quicinc.com, "Ben Skeggs" <bskeggs@redhat.com>,
	"Jonas Karlman" <jonas@kwiboo.se>,
	intel-gfx@lists.freedesktop.org,
	"Ville Syrjala" <ville.syrjala@linux.intel.com>,
	nganji@quicinc.com, linux-amlogic@lists.infradead.org,
	"Vincent Abriou" <vincent.abriou@st.com>,
	"Jernej Skrabec" <jernej.skrabec@siol.net>,
	"Purism Kernel Team" <kernel@puri.sm>,
	jeykumar@quicinc.com, "Seung-Woo Kim" <sw0312.kim@samsung.com>,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	"Icenowy Zheng" <icenowy@aosc.io>
Subject: Re: [PATCH v2 03/17] drm: Nuke mode->vrefresh
Date: Mon, 06 Apr 2020 18:23:39 -0700	[thread overview]
Message-ID: <db6db406ab0c1378fdac427d9112cf97@codeaurora.org> (raw)
In-Reply-To: <87tv1xko9l.fsf@intel.com>

Hi Jani

On 2020-04-06 01:32, Jani Nikula wrote:
> On Fri, 03 Apr 2020, abhinavk@codeaurora.org wrote:
>> On 2020-04-03 13:39, Ville Syrjala wrote:
>>> diff --git a/drivers/gpu/drm/drm_modes.c 
>>> b/drivers/gpu/drm/drm_modes.c
>>> index fec1c33b3045..e3d5f011f7bd 100644
>>> --- a/drivers/gpu/drm/drm_modes.c
>>> +++ b/drivers/gpu/drm/drm_modes.c
>>> @@ -759,9 +759,7 @@ int drm_mode_vrefresh(const struct 
>>> drm_display_mode
>>> *mode)
>>>  {
>>>  	int refresh = 0;
>>> 
>>> -	if (mode->vrefresh > 0)
>>> -		refresh = mode->vrefresh;
>> 
>> The mode->vrefresh has been replaced with calling this API in all its
>> usages.
>> However in this API, the above if statement was returning the vrefresh
>> if it was already
>> set. mode->clock is holding the pixel clock . So this will not cause 
>> any
>> issues in non-compressed cases.
>> In case of compression like DSC, the pixel
>> clock will be different based on the compression ratio hence the
>> mode->clock will change but fps will not.
>> So we did have usages in our downstream driver where we would use this
>> API and the refresh rate
>> returned will be the mode->vrefresh which did not change but after 
>> this
>> change for those cases it will end up returning the refresh rate
>> calculated using mode->clock which will result in a different value 
>> now.
>> So is the recommendation that even in the case of compression
>> mode->clock should always hold
>> uncompressed pixel clock value because with this part of the change we
>> will now get a different value when we call this API.
> 
> Yes. The mode remains the same regardless of compression, and
> compression is just an implementation detail of the transport.
> 
> You may need to maintain separate "physical port clock" and "logical
> port clock" for DSC, where the latter is a function of the former and
> the DSC parameters. And then you can see if your logical port clock
> provides enough bandwidth for your mode. But this is up to your driver
> and encoder implementation.
> 
> BR,
> Jani.

Thanks for the information. We will make changes to our driver to 
accommodate the changes in the
drm_mode_vrefresh API.

Thanks

Abhinav

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic