Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMDGPU blog #54

Closed
wants to merge 1 commit into from
Closed

AMDGPU blog #54

wants to merge 1 commit into from

Conversation

jcfrosty
Copy link
Collaborator

this is my first time attempting to add to the blog. Can someone please double check my work?

@jcfrosty jcfrosty requested a review from Ryuno-Ki June 23, 2017 22:26
Copy link
Collaborator

@Ryuno-Ki Ryuno-Ki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking time to write this article, @jcfrosty!

I found some minor bits (Most the times I commented them once, but they explain to further occurences in this text).

I request changes mainly because I would like to see more level-two-headlines (##), in order to help the reader grasp the content easier. As an example read only the headlines and tell me, what the content is about.

Since the topic is quite technical, you should add link to a wiki (ours or Wikipedia) for technical terms like fgrlx, DC or similiar (my personal opinion).

If you add the headlines, I approve the PR. The other changes just ease reading IMHO.

Currently, there are some gaps in support. Where PRO supports Freesync, HDMI 2.0,
HDMI Audio, OpenCL, etc., the AMDGPU driver will eventually attain all that
through what is known as Display Code (DC) or DAL/HAL (Display/Hardware abstraction
layer). The old DAL was just as it sounds, an abastraction layer. This
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two spaces after the period of the sentence concerning DAL, HAL.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

a HAL was a simple method. They could write code that (mostly) worked for
both Windows and Linux at the same time. So primary focus on Windows translated
to some focus on Linux. Unfortunately, this doesn't mean it worked well.
We all remember how 'lovely' fglrx was a times with broken crossfire support,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please use proper quotation marks? Like , or ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


OK so small software dev team, HAL isn't fully working in this instance,
what is the solution? They decided to go open source. "Great, shouldn't this
be simple?" Short answer, No. They have to go over code with a fine tooth comb
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, please use proper quotation marks: , and .

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

be simple?" Short answer, No. They have to go over code with a fine tooth comb
to make sure they aren't releasing code they "don't own" persay. Can't exactly
opensource DirectX related etc. After that, they also have to remove the HAL and recreate
code from scratch to make a native/direct communication line for the linux kernel.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you used a lowercase Linux. On purpose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

bumped more frequently due to the constant development for AMDGPU.

We understand you may feel some frustration in this time awaiting fixes,
but let it be known this issue is affecting all Linux distributions.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I would need to think about how to interpret the phrase „let it be known” …

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

and performance boosts, but maintaining stability.

We hope you're enjoying Sabayon just as much as we do!
-Frosty
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That line will be placed after the one above. Are you fine with that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@jcfrosty jcfrosty closed this Jun 26, 2017
@Ryuno-Ki
Copy link
Collaborator

Hi, @jcfrosty!

Could you tell me, why you closed this PR?

@jcfrosty
Copy link
Collaborator Author

jcfrosty commented Jun 26, 2017 via email

So when can we expect to see support for all items the display code brings us?
Thats an excellent, but daunting question.
AMD is about to meet the crossroads in which they either:
* Do not support their latest graphics cards at all ,VEGA and beyond, upon release
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

" ,VEGA" → ", VEGA"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

quickly. radv is the radeon opensource vulkan project. Have you tried DOTA2
with vulkan lately? It works. [See the benchmarks and gaming results here.](https://www.phoronix.com/scan.php?page=article&item=amdgpu-new-1710&num=1)

Valve has also offered is services to help forward AMDGPU. [Looking forward to VR.](https://www.phoronix.com/scan.php?page=news_item&px=AMDGPU-PRO-VR-Linux)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is → its

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

quickly. radv is the radeon opensource vulkan project. Have you tried DOTA2
with vulkan lately? It works. [See the benchmarks and gaming results here.](https://www.phoronix.com/scan.php?page=article&item=amdgpu-new-1710&num=1)

Valve has also offered is services to help forward AMDGPU. [Looking forward to VR.](https://www.phoronix.com/scan.php?page=news_item&px=AMDGPU-PRO-VR-Linux)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to make a note what VR is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Thats an excellent, but daunting question.
AMD is about to meet the crossroads in which they either:
* Do not support their latest graphics cards at all ,VEGA and beyond, upon release
* Push their team to get the DC conversion completed and submitted to the linux kernel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be worth to "unwind" the acronym here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Push their team to get the display code converted and submitted to the Linux kernel"

Better but now it reads like a generic display code (so why would it need converting?), not that DC stat stands next to HAL and DAL.

@jcfrosty
Copy link
Collaborator Author

Sorry for the mess. as you can tell, I'm not completely familiar with github yet. I'm learning ;)

#55

@Enlik
Copy link
Member

Enlik commented Jun 26, 2017

General comment: I enjoyed the read but my feel is that the article is more technical (due to vocabulary and style being used), I could said nerdy, and can be harder by the less technical ones, or can leave readers with less knowledge of the video/GPU ecosystem some questions.

By itself, this is not a bad thing, especially given that it's signed by your name (meaning the style /can/ be personal as I think of it), and in my option it could be left as is, but you might keep this in mind when writing futher articles on the website. (I'm not for dumbing down content; reading something like this actually allows someone to learn from it which is great, so take what I wrote with a grain of salt.)

@Enlik
Copy link
Member

Enlik commented Jun 26, 2017

Sorry for the mess. as you can tell, I'm not completely familiar with github yet. I'm learning ;)

This one could be reopened instead, to keep everything in one place. Please do this like this in the future.

@jcfrosty
Copy link
Collaborator Author

Glad you liked it. Yeah, I figured the only people who would be interested in the AMDGPU blog would be people with an AMD card looking for answers (both techy and non). I don't plan on them all being techy, but there are those who are upset with AMD in general and I felt they deserved some sort of explanation. Even if it was a little over the top, they could still get something out of it.

I'll keep in mind the ability to reopen next time. I was looking at the closed image at the top and looking to see if I could click it and get it to reopen, but it doesn't work that way. I just noticed the "reopen and comment" button on the bottom. I should have checked that before opening a new one.

@Enlik Enlik mentioned this pull request Jun 26, 2017
@Enlik
Copy link
Member

Enlik commented Jun 26, 2017

Changing "radeon" to something like "the radeon driver" should make it much easier for those non-AMD device owners in the following sentence.

"Some of you may still be using radeon on
+AMD HD7XXX or R9 2XX (SI/CIK) video cards."

@jcfrosty
Copy link
Collaborator Author

Good point, also fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants