Regards
Shashank
On 10/15/2018 6:42 PM, Jani Nikula wrote:
On Fri, 12 Oct 2018, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
On Sat, Oct 13, 2018 at 12:02:25AM +0530, Sharma, Shashank wrote:
+void lspcon_ycbcr420_config(struct drm_connector *connector,
+ struct intel_crtc_state *crtc_state)
+{
+ const struct drm_display_info *info = &connector->display_info;
+ const struct drm_display_mode *adjusted_mode =
+ &crtc_state->base.adjusted_mode;
+
+ if (drm_mode_is_420_only(info, adjusted_mode) &&
+ connector->ycbcr_420_allowed) {
+ crtc_state->port_clock /= 2;
This looks bogus. We're talking about DP here so we should not be
frobbing port_clock. And since we output 4:4:4 from the port
anyway we don't even have to take 4:2:0 into consideration
when calculating port_clock.
I agree on this.
Ah, this guy is called before we even calculate port_clock.
That explains why this didn't cause any problems. So this is
just dead code here.
And looks like the port_clock adjustment in intel_hdmi_ycbcr420_config()
also dead code since the caller overwrites it there as well.
I am not sure if that's the case for Native HDMI 2.0.
In intel_hdmi_420_config we are updating the config->port_clock, which
is the same thing being updated for 8/12/16 BPC deep color too, just
after this function.
I have tested the output with HDMI 2.0 analyzer, which reported right
pixel clock (297Mhz). Now, as any of the port clock calculations do not
get information
about HDMI output type, there is no other way they will calculate the
right pixel clock (594/2) for 4:2:0 outputs. So I believe that code is
active.
It effectively does this:
1. port_clock = who knows
2. port_clock /= 2;
Here, in intel_hdmi_compute_ycbcr_config() function, we are additionally
doing this also:
clock_8bpc /= 2;
clock_10bpc /= 2;
clock_12bpc /= 2;
3. if (12bpc)
port_clock = clock_12bpc;
else if (10bpc)
port_clock = clock_10bpc;
else
port_clock = clock_8bpc;
This means effectively the selected clock is /2, so the code is not dead
(thankfully :-))
Only the port_clock/=2 part. I never claimed the rest was dead.
The whole series pushed to dinq, dismissing this last part with Ville's
approval, to get some closure here. Thanks for the patches and
review. Further patches to clean up the existing and new dead code will
be appreciated.
Thanks Jani, I will have a look at the series post merge, and send any
follow up / cleanup patches if required.
- Shashank
BR,
Jani.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
